From patchwork Fri Oct 29 21:41:03 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 69632 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0CB25B70D4 for ; Sat, 30 Oct 2010 08:41:51 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758475Ab0J2Vl0 (ORCPT ); Fri, 29 Oct 2010 17:41:26 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:42796 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755194Ab0J2VlZ (ORCPT ); Fri, 29 Oct 2010 17:41:25 -0400 Received: from mail-iw0-f174.google.com (mail-iw0-f174.google.com [209.85.214.174]) (authenticated bits=0) by smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with ESMTP id o9TLfObB027838 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=FAIL); Fri, 29 Oct 2010 14:41:24 -0700 Received: by iwn10 with SMTP id 10so4267867iwn.19 for ; Fri, 29 Oct 2010 14:41:23 -0700 (PDT) Received: by 10.42.183.11 with SMTP id ce11mr9927334icb.318.1288388483705; Fri, 29 Oct 2010 14:41:23 -0700 (PDT) MIME-Version: 1.0 Received: by 10.231.14.134 with HTTP; Fri, 29 Oct 2010 14:41:03 -0700 (PDT) In-Reply-To: <20101029.125920.189692530.davem@davemloft.net> References: <20101029.125920.189692530.davem@davemloft.net> From: Linus Torvalds Date: Fri, 29 Oct 2010 14:41:03 -0700 Message-ID: Subject: Re: [GIT] Networking To: David Miller Cc: akpm@linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org X-Spam-Status: No, hits=-3.428 required=5 tests=AWL, BAYES_00, OSDL_HEADER_SUBJECT_BRACKETED X-Spam-Checker-Version: SpamAssassin 3.2.4-osdl_revision__1.47__ X-MIMEDefang-Filter: lf$Revision: 1.188 $ X-Scanned-By: MIMEDefang 2.63 on 140.211.169.13 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Oct 29, 2010 at 12:59 PM, David Miller 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;