linuxolator: proc/filesystems and sysfs function implementations

[Scot Hetzel]

On 1/15/07, Alexander Leidinger <Alexander at leidinger.net> wrote:
> Quoting Scot Hetzel <swhetzel at gmail.com> (from Sun, 14 Jan 2007
> 14:54:28 -0600):
>
> > On 1/14/07, Divacky Roman <xdivac02 at stud.fit.vutbr.cz> wrote:
>
> >> +                       buf = (char *)(uintptr_t)args->arg2;
> > :
> >> +                       bsd_to_linux_fs(vfsp->vfc_name, &fs);
> >> +                       len = strlen(fs.name) + 1;
> >> +                       error = copyout(fs.name, buf, len);
> >>
> >> no need to check if it fits in the userspace buffer? also you
> >> dont check return value
> >>
> >
> > According to the linux man page, there isn't a way to check the size
> > of buf, as we are only passed the value of the pointer in args->arg2.
>
> Ugh... there's not even an implicit size because of a fixed definition
> of the target buffer in some header? That would be very bad design.
> The kernel could overwrite userland data even if the kernel is not
> malicious (load a new FS with a too long name and BOOOOM).
>
> > Should I replace buf with  (char *)(uintptr_t)args->arg2 in the
> > copyout, since it is only used there.
> >
> > http://www.die.net/doc/linux/man/man2/sysfs.2.html
>
> ---snip---
> BUGS
> There is no libc or glibc support. There is no way to guess how large
> buf should be.
> ---snip---
>
> That's bad. That's very very bad. I don't like this in the FreeBSD
> kernel, I want an upper bound. Would you please try to find some
> artificial upper bound? The MFSNAMELEN would be one candidate. A
> better candidate would be a similar Linux value.
>

The patch is using MFSNAMELEN as the upper bound.

> >> also... you strcpy string to another string, are you sure it always
> >>  fits? how do you
> >> asure that?
> >>
> >
> > name can't be larger than MFSNAMELEN, and sizeof(fs->name) == MFSNAMELEN.
>
> Please use strlcpy with the sizeof (defensive programming).
>
Changed strcpy to strlcpy.

> > 16 is the value of MFSNAMELEN and used by the vfsconf struct to define
> > the max size of vfc_name.  I kept name (in the linux_file_system_type
>
> Is there a similar value in Linux? It would be better to use this if
> it exists.
>
> > struct) the same size, so that we didn't have to worry about the size
> > when using strcpy.
>
> When you find a value like MFSNAMELEN on Linux, please have a look at
> linux_misc.c:linux_prctl(), specially the LINUX_PR_{SET|GET}_NAME case
> and the comments in there. What you are doing here should be done
> similar.
>
I wasn't able to locate a value like MFSNAMELEN.  I looked at other
implementations and the SGI version uses FSTYPESZ  (same value as
MFSNAMELEN), to limit the size of the buffer.

The attached patch is using MFSNAMELEN to limit the size copied in
(LINUX_GETFSIND) and copied out (LINUX_GETFSTYPE).  LINUX_GETFSTYPE
also assumes that the buffer we are copying too is >= MFSNAMELEN.

I looked at the LTP sysfs02.c test, and in that file the buffer is set
to 40 bytes.

Attached is the updated patch, all of the code is now in
linux_misc.{c,h}, linprocfs.c and linux_util.h (define for
bsd_to_linux_fs function).

Scot
-- 
DISCLAIMER:
No electrons were mamed while sending this message. Only slightly bruised.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sysfs3.patch
Type: text/x-diff
Size: 7092 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-emulation/attachments/20070126/bc41ecdd/sysfs3.bin