[statfs12] Re: CVS commit: src

[Maxime Villard <max%m00nbsd.net@localhost>]

> Module Name:    src
> Committed By:   christos
> Date:           Fri Oct  4 01:28:03 UTC 2019
>
> Modified Files:
>         src/lib/libc/compat/sys: compat_statfs.c
>         src/sys/compat/common: vfs_syscalls_20.c
>         src/sys/compat/sys: mount.h
>
> Log Message:
> deduplicate the conversion function from statvfs -> statfs12
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.8 -r1.9 src/lib/libc/compat/sys/compat_statfs.c
> cvs rdiff -u -r1.43 -r1.44 src/sys/compat/common/vfs_syscalls_20.c
> cvs rdiff -u -r1.10 -r1.11 src/sys/compat/sys/mount.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

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?

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.

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.

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.

As I said, please revert all of this change, it is just plain wrong and
hasn't received any testing.

Thank you,
Maxime