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);
>
