netdev - Re: [GIT] Networking

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] [day] [month] [year] [list]

Message-Id: <20101030.164727.28829030.davem@davemloft.net>
Date:	Sat, 30 Oct 2010 16:47:27 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	torvalds@...ux-foundation.org
Cc:	akpm@...ux-foundation.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [GIT] Networking

From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Fri, 29 Oct 2010 14:41:03 -0700

> On Fri, Oct 29, 2010 at 12:59 PM, David Miller <davem@...emloft.net> 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.

Done, thanks.

> 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.

I honestly don't think it matters.

I suppose we could put the access_ok() check right before these
single-buffer truncations, and then also in the per-iovec check of
{compat_}verify_iovec().

But what would all of that really give us?

Ingrained in datagram socket handling is the idea that the whole
buffer will be processed, and for stream sockets partial buffer
transfers are OK.

And I think this aligns with how we implement and check things
right now.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html