Thread (35 messages) 35 messages, 8 authors, 2011-08-04

Re: [PATCH] net: Fix security_socket_sendmsg() bypass problem.

From: Anton Blanchard <hidden>
Date: 2011-08-03 13:29:57
Subsystem: networking [general], networking [sockets], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima, Willem de Bruijn, Linus Torvalds

Hi,
quoted
I much prefer to make the error handling more correct, rather than
making sendmmsg() have fundamentally different semantics depending
upon the underlying LSM.
Well, the way how sendmmsg() returns error code is tricky. But
recvmmsg() has been doing in this way for a while. So, for symmetry
reason, maybe sendmmsg() should do as with recvmmsg() since it is too
late to change recvmmsg()'s way.

So, programmers should be warned (in the man pages) that they should
always call getsockopt(SO_ERROR) (in order to clear the error code)
if sendmmsg() or recvmmsg() returned less than requested.
As you suggest, I wanted to mirror how recvmmsg returns errors. But I
now agree with Dave, we should not return an error if we managed to send
any datagrams.

Perhaps we need to modify recvmmsg to do the same?
By the way, don't we want integer overflow check and/or
cond_resched() here? I don't know whether there is an arch where
userspace can allocate (1 << BITS_PER_INT) * sizeof(struct msghdr)
bytes using malloc() and kernel can allocate huge memory for the
socket buffer.

#include <stdio.h>
int main(int argc, char *argv[])
{
        int datagrams = 0;
        unsigned int vlen = 4294967290U;
        while (datagrams < vlen)
                datagrams++;
        printf("%u\n", datagrams);
        return 0;
}

I think this program (on x86_32) will print an IS_ERR() value upon
success.
Good catch. I wonder if we can do something similar to read/write where
we just truncate the length. What value should we use? One option is to
reuse UIO_MAXIOV (1024).

The following patch is compiled tested only so far.

Anton
--

[PATCH] net: Cap number of elements for recvmmsg and sendmmsg 

To limit the amount of time we can spend in recvmmsg and sendmmsg,
cap the number of elements to UIO_MAXIOV (currently 1024). 
       
Signed-off-by: Anton Blanchard <redacted>
Cc: <stable@kernel.org>
---
diff --git a/net/socket.c b/net/socket.c
index b1cbbcd..ad345b1 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1999,6 +1999,9 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 	struct compat_mmsghdr __user *compat_entry;
 	struct msghdr msg_sys;
 
+	if (vlen > UIO_MAXIOV)
+		vlen = UIO_MAXIOV;
+
 	datagrams = 0;
 
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
@@ -2199,6 +2202,9 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 	struct msghdr msg_sys;
 	struct timespec end_time;
 
+	if (vlen > UIO_MAXIOV)
+		vlen = UIO_MAXIOV;
+
 	if (timeout &&
 	    poll_select_set_timeout(&end_time, timeout->tv_sec,
 				    timeout->tv_nsec))
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help