>
> Please revert all of this change.
>
> First, there was a clear vulnerability in this change, which I fixed in:
>
> https://mail-index.netbsd.org/source-changes/2020/06/27/msg118731.html
>
> Then, as I said in the change, there are additional problems:
>
> 137 static __inline int
> 138 statvfs_to_statfs12_copy(const void *vs, void *vs12, size_t l)
> 139 {
> 140 struct statfs12 *s12 = STATVFSBUF_GET();
> 141 int error;
> 142
> 143 statvfs_to_statfs12(vs, s12);
> 144 error = copyout(s12, vs12, l);
> 145 STATVFSBUF_PUT(s12);
> 146
> 147 return error;
> 148 }
>
> STATVFSBUF_GET() allocates struct statvfs, but here we're using struct
> statfs12. How can this be expected to be correct?
It is larger than needed, so it works.
> Then the copyout is done with a size, and again there are problems here.
> In compat_20_sys_getfsstat() the size given is struct statvfs90, which
> matches neither the filled size nor the allocated size.
That is a mistake and I have fixed it.
> The other callers have even bigger problems. For example
> compat_20_sys_statfs() passes zero as size. So the result simply never
> gets copied out. How can this be expected to be correct? As far as I can
> tell the syscall simply cannot work now.
Same bug as above.
> Finally, I don't even understand what this change dedups. It just moved
> the functions from one place to another, introduced bugs in them, but
> didn't reduce the code size in any way.
It reduces maintainability, since the conversion was done in two places
(in libc and the kernel) and now it is done in one.
> As I said, please revert all of this change, it is just plain wrong and
> hasn't received any testing.
I have fixed it instead, if you find more bugs please let me know.
christos
Attachment:
signature.asc
Description: Message signed with OpenPGP