| 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
| ||
|
Date: Mon, 26 Nov 2018 11:46:10 +0000
From: Slavomir Kaslev <kaslevs@...are.com>
To: Al Viro <viro@...iv.linux.org.uk>
CC: syzbot <syzbot+ce18da013d76d837144d@...kaller.appspotmail.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"kgraul@...ux.ibm.com" <kgraul@...ux.ibm.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"stranche@...eaurora.org" <stranche@...eaurora.org>,
"syzkaller-bugs@...glegroups.com" <syzkaller-bugs@...glegroups.com>
Subject: Re: WARNING in csum_and_copy_to_iter
On Sun, Nov 25, 2018 at 3:52 AM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> On Sat, Nov 24, 2018 at 09:44:36PM +0000, Al Viro wrote:
>
> > No point, IMO - the fix isn't hard and bisect hazard created by the whole thing
> > is both mild (spurious WARN() in case that used to fail anyway) _and_ won't
> > disappear from reverting, obviously. I'll post a fix later tonight...
>
> FWIW, I think the following ought to work; it's obviously a pair of commits
> (introduction of convenience helper/switch to its use + csum_and_copy_to_iter()
> for ITER_PIPE), as well as commit message, etc., but I would really appreciate
> if folks gave it a look _and_ a beating.
Tested the patch in qemu, splice reading from udp and vsock sockets (with
https://github.com/skaslev/thru), and it seems to work great.
No warnings or suspicious messages in dmesg with kernel config similar to what
syzbot is using
https://github.com/google/syzkaller/blob/master/docs/linux/kernel_configs.md
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> ---
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 7ebccb5c1637..621984743268 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -560,6 +560,44 @@ static size_t copy_pipe_to_iter(const void *addr, size_t bytes,
> return bytes;
> }
>
> +static __wsum csum_and_memcpy(void *to, const void *from, size_t len,
> + __wsum sum, size_t off)
> +{
> + __wsum next = csum_partial_copy_nocheck(from, to, len, 0);
> + return csum_block_add(sum, next, off);
> +}
> +
> +static size_t csum_and_copy_to_pipe_iter(const void *addr, size_t bytes,
> + __wsum *csum, struct iov_iter *i)
> +{
> + struct pipe_inode_info *pipe = i->pipe;
> + size_t n, r;
> + size_t off = 0;
> + __wsum sum = *csum;
> + int idx;
> +
> + if (!sanity(i))
> + return 0;
> +
> + bytes = n = push_pipe(i, bytes, &idx, &r);
> + if (unlikely(!n))
> + return 0;
> + for ( ; n; idx = next_idx(idx, pipe), r = 0) {
> + size_t chunk = min_t(size_t, n, PAGE_SIZE - r);
> + char *p = kmap_atomic(pipe->bufs[idx].page);
> + sum = csum_and_memcpy(p + r, addr, chunk, sum, off);
> + kunmap_atomic(p);
> + i->idx = idx;
> + i->iov_offset = r + chunk;
> + n -= chunk;
> + off += chunk;
> + addr += chunk;
> + }
> + i->count -= bytes;
> + *csum = sum;
> + return bytes;
> +}
> +
> size_t _copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i)
> {
> const char *from = addr;
> @@ -1368,17 +1406,15 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum,
> err ? v.iov_len : 0;
> }), ({
> char *p = kmap_atomic(v.bv_page);
> - next = csum_partial_copy_nocheck(p + v.bv_offset,
> - (to += v.bv_len) - v.bv_len,
> - v.bv_len, 0);
> + sum = csum_and_memcpy((to += v.bv_len) - v.bv_len,
> + p + v.bv_offset, v.bv_len,
> + sum, off);
> kunmap_atomic(p);
> - sum = csum_block_add(sum, next, off);
> off += v.bv_len;
> }),({
> - next = csum_partial_copy_nocheck(v.iov_base,
> - (to += v.iov_len) - v.iov_len,
> - v.iov_len, 0);
> - sum = csum_block_add(sum, next, off);
> + sum = csum_and_memcpy((to += v.iov_len) - v.iov_len,
> + v.iov_base, v.iov_len,
> + sum, off);
> off += v.iov_len;
> })
> )
> @@ -1412,17 +1448,15 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum,
> 0;
> }), ({
> char *p = kmap_atomic(v.bv_page);
> - next = csum_partial_copy_nocheck(p + v.bv_offset,
> - (to += v.bv_len) - v.bv_len,
> - v.bv_len, 0);
> + sum = csum_and_memcpy((to += v.bv_len) - v.bv_len,
> + p + v.bv_offset, v.bv_len,
> + sum, off);
> kunmap_atomic(p);
> - sum = csum_block_add(sum, next, off);
> off += v.bv_len;
> }),({
> - next = csum_partial_copy_nocheck(v.iov_base,
> - (to += v.iov_len) - v.iov_len,
> - v.iov_len, 0);
> - sum = csum_block_add(sum, next, off);
> + sum = csum_and_memcpy((to += v.iov_len) - v.iov_len,
> + v.iov_base, v.iov_len,
> + sum, off);
> off += v.iov_len;
> })
> )
> @@ -1438,8 +1472,12 @@ size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum,
> const char *from = addr;
> __wsum sum, next;
> size_t off = 0;
> +
> + if (unlikely(iov_iter_is_pipe(i)))
> + return csum_and_copy_to_pipe_iter(addr, bytes, csum, i);
> +
> sum = *csum;
> - if (unlikely(iov_iter_is_pipe(i) || iov_iter_is_discard(i))) {
> + if (unlikely(iov_iter_is_discard(i))) {
> WARN_ON(1); /* for now */
> return 0;
> }
> @@ -1455,17 +1493,15 @@ size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum,
> err ? v.iov_len : 0;
> }), ({
> char *p = kmap_atomic(v.bv_page);
> - next = csum_partial_copy_nocheck((from += v.bv_len) - v.bv_len,
> - p + v.bv_offset,
> - v.bv_len, 0);
> + sum = csum_and_memcpy(p + v.bv_offset,
> + (from += v.bv_len) - v.bv_len,
> + v.bv_len, sum, off);
> kunmap_atomic(p);
> - sum = csum_block_add(sum, next, off);
> off += v.bv_len;
> }),({
> - next = csum_partial_copy_nocheck((from += v.iov_len) - v.iov_len,
> - v.iov_base,
> - v.iov_len, 0);
> - sum = csum_block_add(sum, next, off);
> + sum = csum_and_memcpy(v.iov_base,
> + (from += v.iov_len) - v.iov_len,
> + v.iov_len, sum, off);
> off += v.iov_len;
> })
> )
Powered by blists - more mailing lists