[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <64c9174fda48e_1bf0a42945f@willemb.c.googlers.com.notmuch>
Date: Tue, 01 Aug 2023 10:31:43 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: David Howells <dhowells@...hat.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: dhowells@...hat.com, Jakub Kicinski <kuba@...nel.org>,
syzbot <syzbot+f527b971b4bdc8e79f9e@...kaller.appspotmail.com>,
bpf@...r.kernel.org, brauner@...nel.org, davem@...emloft.net,
dsahern@...nel.org, edumazet@...gle.com,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, pabeni@...hat.com,
syzkaller-bugs@...glegroups.com, viro@...iv.linux.org.uk
Subject: Re: Endless loop in udp with MSG_SPLICE_READ - Re: [syzbot] [fs?]
INFO: task hung in pipe_release (4)
David Howells wrote:
> The attached seems to work. I still think copy isn't correctly calculated in
> some circumstances - as I showed, several terms in the maths cancel out,
> including the length of the data.
That arithmetic makes assumptions that are specific to a particular
set of conditions (e.g., that pagedlen is non-zero).
Since the arithmetic is so complicated and error prone, I would try
to structure a fix that is easy to reason about to only change
behavior for the MSG_SPLICE_PAGES case.
> I'm also not entirely sure what 'paged' means in this function. Should it
> actually be set in the MSG_SPLICE_PAGES context?
I introduced it with MSG_ZEROCOPY. It sets up pagedlen to capture the
length that is not copied.
If the existing code would affect MSG_ZEROCOPY too, I expect syzbot
to have reported that previously.
> ---
> udp: Fix __ip_addend_data()
>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 6e70839257f7..54675a4f2c9f 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1157,7 +1157,7 @@ static int __ip_append_data(struct sock *sk,
> pskb_trim_unique(skb_prev, maxfraglen);
> }
>
> - copy = datalen - transhdrlen - fraggap - pagedlen;
> + copy = max_t(int, datalen - transhdrlen - fraggap - pagedlen, 0);
> if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) {
> err = -EFAULT;
> kfree_skb(skb);
>
Powered by blists - more mailing lists