On 26 November 2012 19:49, Rich Felker <[email protected]> wrote: > 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
This is just for backwards compatibility. This patch tries to use the existing structures so we don't have to avoid a new one for these architectures. As for the simple "buf" pointer check, I think you missed the point. This pointer is never passed to the kernel (it is not passed to the INLINE_SYSCALL macro) so the kernel does not know about it. It is possible your userspace program to pass an invalid "buf" pointer and the INLINE_SYSCALL to succeed but you will crash after that when you try to dereference any member of the invalid "buf" pointer. -- Regards, Markos _______________________________________________ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
