netdev - Re: [PATCH] net: Limit socket I/O iovec total length to INT

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

Message-ID: <AANLkTi=sUDRzGnkp57OCQUi9APHWLYLShhiTVHLyq1eV@mail.gmail.com>
Date:	Fri, 29 Oct 2010 09:21:16 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	jon.maloy@...csson.com, allan.stephens@...driver.com,
	Dan Rosenberg <drosenberg@...curity.com>
Subject: Re: [PATCH] net: Limit socket I/O iovec total length to INT_MAX.

On Fri, Oct 29, 2010 at 8:28 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> We do need to fix the readv/writev path, though. It does the
> rw_verify_area(), but it doesn't seem to limit the size to the
> returned length, but still uses the original one. Hmm.
>
> I think I'll take care of the readv/writev thing, and send it by Al to verify.

Ok, something like the attached should do the same as the suggested
networking iovec verification (except the VFS code considers the iov
'len' to be a ssize_t, and a negative value to be an error, and claims
that that is what SuS says. I don't much care, since we ignore SuS wrt
the overflow anyway, and now limit IO to MAX_RW_COUNT the same way we
do for regular reads and writes, but it's possibly worth thinking
about unifying the networking verify_iovec and the VFS layer one)

Al? What do you think? You've not seen some of the background, but it
all boils down to simple "some protocols overflow in 'int'", the exact
same way we had filesystems that had int counters for access lengths
etc. So for the same reason we did the MAX_RW_COUNT thing for regular
read/write calls, the networking code is now going to do it for all
the sendmsg etc iovec things.

Even outside of any networking issues, this unifies the handling of
read/write vs readv/writev in the "cap IO size" area, so I think we
should have done this originally (the readv/writev cases actually
already did the capping by calling rw_verify_area(), but then the code
threw away the capped value, and only looked at the error return, and
used the uncapped size)

NOTE! UNTESTED! It compiles, but I didn't actually boot to check.
Maybe it has some stupid thinko in it. Also, the patch ended up being
a bit bigger than it needed to be, because I found some annoying
whitespace issues in rw_copy_check_uvector (spaces at the beginning of
lines that had tabs in them) that I couldn't keep myself from not
fixing.

Comments?

                                          Linus

View attachment "patch.diff" of type "text/x-patch" (4756 bytes)