Re: buffer overflow in /proc/sys/sunrpc/transports

On Sat, Aug 30, 2008 at 9:06 PM, Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote:
[Vegard Nossum - Sat, Aug 30, 2008 at 08:44:22PM +0200]
| Hi,
|
| I noticed that something weird is going on with /proc/sys/sunrpc/transports.
| This file is generated in net/sunrpc/sysctl.c, function proc_do_xprt(). When
| I "cat" this file, I get the expected output:
|
| $ cat /proc/sys/sunrpc/transports
| tcp 1048576
| udp 32768
|
| But I think that it does not check the length of the buffer supplied by
| userspace to read(). With my original program, I found that the stack was
| being overwritten by the characters above, even when the length given to
| read() was just 1. So I have created a test program, see it at the bottom of
| this e-mail. Here is its output:
|
...

Indeed, maybe just add checking for user buffer length?
As proc_dodebug() in this file are doing. I don't think
the user would be happy with his stack burned :)

Something like:
---

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;



BTW, look at this:

$ od -A x -t x1z /proc/sys/sunrpc/transports
000000 74 63 70 20 31 30 34 38 35 37 36 0a 75 64 70 20 >tcp 1048576.udp <
000010 33 32 37 36 38 0a 00 00 00 00 00 00 00 00 00 00 >32768...........<
000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >................<
*
0003e0 00 00 00 00 00 00 00 00 00 00 >..........<
0003ea

...and:

$ strace -e trace=read cat /proc/sys/sunrpc/transports > /dev/null
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0@G\316E4\0\0\0"...,
512) = 512
read(3, "tcp 1048576\nudp 32768\n\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4074
read(3, "", 4096) = 0

...why does it have a huge return value? The output is only about 40
bytes... why add all the \0? Would your patch also fix this?


Vegard

--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: Internal TCP/IP send buffer?
    ... > quickly therefore I need to minimize buffering on both the sender and the ... Then TCP is the wrong protocol for you. ... What is this buffer, and what can I do to work ... TCP to be made to behave like UDP. ...
    (microsoft.public.win32.programmer.networks)
  • Re: buffer overflow in /proc/sys/sunrpc/transports
    ... I get the expected output: ... | tcp 1048576 ... | udp 32768 ... maybe just add checking for user buffer length? ...
    (Linux-Kernel)
  • Re: Winsock2 SO_SNDBUF / UDP message send process
    ... There's buffer size. ... and Winsock hasn't had time to send the first 50 bytes datagram? ... As about internals, TCP stack ... UDP doesn't wait for ACK opposite to TCP ...
    (microsoft.public.win32.programmer.networks)
  • TCP vs. UDP receive Buffer.
    ... I like to underestand the difference between a TCP vs. UDP received ... layer AFD.sys.. ... I'm trying to capture a receive UDP and pass it to a higher layer app ... Is this mean that in case of TDI_EVENT_RECEIVE the buffer is supplied by ...
    (microsoft.public.development.device.drivers)
  • Re: Coordinating TCP projects
    ... It is rapidly becoming clear that quite a few of us have Big Plans for the TCP implementation over the next 12-18 months. ... Jim and I recently discussed the idea of implementing autotuning of the TCP reassembly queue size based on analysis of some experimental work we've been doing. ... This means that if the TCP window grows to be more than 48 segments wide, and a packet is lost, the receiver will buffer the next 48 segments in the reassembly queue and subsequently drop all the remaining segments in the window because the reassembly buffer is full i.e. 1 packet loss in the network can equate to many packet losses at the receiver because of insufficient buffering. ... We observed that the socket receive buffer size provides a good indication of the expected number of bytes in flight for a connection, and can therefore serve as the figure to base the size of the reassembly queue on. ...
    (freebsd-arch)