[PATCH v2 28/46] statfs: Use statfs64 if arch does not have the statfs syscall

Rich Felker dalias at aerifal.cx
Mon Nov 26 19:49:48 UTC 2012


On Mon, Nov 26, 2012 at 02:31:06PM -0500, Mark Salter wrote:
> On Mon, 2012-11-26 at 14:24 +0000, Markos Chandras wrote:
> > +int __libc_statfs(const char *path, struct statfs *buf)
> > +{
> > +       struct statfs64 b;
> > +       int err;
> > +
> > +       /*
> > +        * See if pointer has a sane value.
> > +        * This does not prevent the user from
> > +        * passing an arbitrary possitive value
> > +        * that can lead to a segfault or potential
> > +        * security problems
> > +        */
> > +
> > +       if (buf == NULL || (int)buf < 0) {
> > +               __set_errno(EFAULT);
> > +               return -1;
> > +       }
> 
> This seems wrong. Doesn't the kernel already validate addresses passed
> in from userspace. Even in the no-MMU case, some architectures add
> basic checking for user addresses.
> 
> In any case, the "(int)buf < 0" is clearly non-portable. C6X can have
> perfectly good addresses which make negative ints.

Indeed, this code is definitely wrong as-written. There's no
obligation to fail with EFAULT when a bad address is given, so just
crashing is perfectly acceptable. 

With that said, I question why this code is even needed. If an arch
doesn't have a non-64-bit statfs call, then why would it have a
separate non-64-bit, legacy statfs structure? If it really needs one,
the statfs structure could just be defined to match the layout of the
statfs64 structure, but with 32-bit off_t fields and 32 bits of
padding next to them instead of the usual 64-bit off_t. Then no
conversion code would be needed.

Rich


More information about the uClibc mailing list