Re: [GIT] Networking

[Linus Torvalds]

On Fri, Oct 29, 2010 at 12:59 PM, David Miller <davem@xxxxxxxxxxxxx> wrote:
>
> This has the verify_iovec() INT_MAX limiter change as well as:

I think you'd want this as well, to make sure that sendto/recvfrom
don't generate invalid iovecs.

Feel free to add my sign-off (or just commit it as yourself) after
giving it some testing.

NOTE! On thing that struck me is that the VFS layer does the
"access_ok()" on the pre-truncated size and pointer pair, and I think
that is the correct thing to do. However, the socket layer (and this
patch) just truncates the size, so even if the copy is then done
correctly with the proper user access checking, it will not check that
the whole original buffer was valid - only that the buffer it fills in
is valid.

Now, this is not a security issue (since we're just not checking stuff
that isn't getting filled in), but I think it's a QoI issue - it
allows users to successfully pass in bogus buffers with huge sizes,
and then if the thing only reads a few bytes it will all be ok.

That's not a new thing: the old code may not have truncated the sizes,
but if you pass in a 2GB buffer size, 99.999% of all socket read calls
obviously won't ever fill that 2GB, but will happily return with
whatever is there in the socket now (especially with nonblocking IO
etc). But I do wonder if we shouldn't do the access_ok() on the whole
buffer, as a way to keep user code honest.

                  Linus
 net/socket.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 5247ae1..3ca2fd9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1652,6 +1652,8 @@ SYSCALL_DEFINE6(sendto, int, fd, void __user *, buff, size_t, len,
 	struct iovec iov;
 	int fput_needed;
 
+	if (len > INT_MAX)
+		len = INT_MAX;
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
 	if (!sock)
 		goto out;
@@ -1709,6 +1711,8 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
 	int err, err2;
 	int fput_needed;
 
+	if (size > INT_MAX)
+		size = INT_MAX;
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
 	if (!sock)
 		goto out;