linux-kernel - Re: buffer overflow in /proc/sys/sunrpc/transports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 30 Aug 2008 22:55:51 +0000 (UTC)
From:	daw@...berkeley.edu (David Wagner)
To:	linux-kernel@...r.kernel.org
Subject: Re: buffer overflow in /proc/sys/sunrpc/transports

Cyrill Gorcunov  wrote:
>Index: linux-2.6.git/net/sunrpc/sysctl.c
>===================================================================
>--- linux-2.6.git.orig/net/sunrpc/sysctl.c	2008-07-20 11:40:14.000000000 +0400
>+++ linux-2.6.git/net/sunrpc/sysctl.c	2008-08-30 23:05:30.000000000 +0400
>@@ -69,6 +69,8 @@ static int proc_do_xprt(ctl_table *table
> 		return -EINVAL;
> 	else {
> 		len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
>+		if (*lenp < len)
>+			return -EFAULT;
> 		if (!access_ok(VERIFY_WRITE, buffer, len))
> 			return -EFAULT;

1. Would it be better to use copy_to_user() rather than
access_ok() followed immediately by __copy_to_user()?

2. Is it OK to dereference *lenp directly?  Is lenp a pointer into user
memory or kernel memory?  If it points to user memory, why is it safe to
dereference it directly?  (What about TOCTTOU bugs?)  Should there be
some sparse annotations here to ensure the code is not dereferencing
user pointers directly?  Later on, proc_do_xprt() also dereferences
*lenp and *ppos directly.

3. 'len' is declared as a signed int.  len will be converted to size_t
before doing the comparison, so if len can ever be negative (e.g.,
svc_print_xprts() returns -1 because of an error), this patch will do
the wrong thing.  Looks like the current definition of svc_print_xprts()
won't ever do that, as that code currently stands, so at present this
is not a bug.  However from a security point of view there are benefits
to code whose correctness is 'locally obvious', all else being equal.
In particular this seems like a possible maintenance hazard.  Would it be
better to use type size_t for lengths like this that are never supposed
to be negative?

4. Is proc_dostring() relevant here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/