Commit ce650a16 authored by David Howells's avatar David Howells Committed by Paolo Abeni

udp6: Fix __ip6_append_data()'s handling of MSG_SPLICE_PAGES

__ip6_append_data() can has a similar problem to __ip_append_data()[1] when
asked to splice into a partially-built UDP message that has more than the
frag-limit data and up to the MTU limit, but in the ipv6 case, it errors
out with EINVAL.  This can be triggered with something like:

        pipe(pfd);
        sfd = socket(AF_INET6, SOCK_DGRAM, 0);
        connect(sfd, ...);
        send(sfd, buffer, 8137, MSG_CONFIRM|MSG_MORE);
        write(pfd[1], buffer, 8);
        splice(pfd[0], 0, sfd, 0, 0x4ffe0ul, 0);

where the amount of data given to send() is dependent on the MTU size (in
this instance an interface with an MTU of 8192).

The problem is that the calculation of the amount to copy in
__ip6_append_data() goes negative in two places, but a check has been put
in to give an error in this case.

This happens because when pagedlen > 0 (which happens for MSG_ZEROCOPY and
MSG_SPLICE_PAGES), the terms in:

        copy = datalen - transhdrlen - fraggap - pagedlen;

then mostly cancel when pagedlen is substituted for, leaving just -fraggap.

Fix this by:

 (1) Insert a note about the dodgy calculation of 'copy'.

 (2) If MSG_SPLICE_PAGES, clear copy if it is negative from the above
     equation, so that 'offset' isn't regressed and 'length' isn't
     increased, which will mean that length and thus copy should match the
     amount left in the iterator.

 (3) When handling MSG_SPLICE_PAGES, give a warning and return -EIO if
     we're asked to splice more than is in the iterator.  It might be
     better to not give the warning or even just give a 'short' write.

 (4) If MSG_SPLICE_PAGES, override the copy<0 check.

[!] Note that this should also affect MSG_ZEROCOPY, but that will return
-EINVAL for the range of send sizes that requires the skbuff to be split.
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: David Ahern <dsahern@kernel.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: netdev@vger.kernel.org
Link: https://lore.kernel.org/r/000000000000881d0606004541d1@google.com/ [1]
Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/r/1580952.1690961810@warthog.procyon.org.ukSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 6abce66b
...@@ -1693,7 +1693,10 @@ static int __ip6_append_data(struct sock *sk, ...@@ -1693,7 +1693,10 @@ static int __ip6_append_data(struct sock *sk,
fraglen = datalen + fragheaderlen; fraglen = datalen + fragheaderlen;
copy = datalen - transhdrlen - fraggap - pagedlen; copy = datalen - transhdrlen - fraggap - pagedlen;
if (copy < 0) { /* [!] NOTE: copy may be negative if pagedlen>0
* because then the equation may reduces to -fraggap.
*/
if (copy < 0 && !(flags & MSG_SPLICE_PAGES)) {
err = -EINVAL; err = -EINVAL;
goto error; goto error;
} }
...@@ -1744,6 +1747,8 @@ static int __ip6_append_data(struct sock *sk, ...@@ -1744,6 +1747,8 @@ static int __ip6_append_data(struct sock *sk,
err = -EFAULT; err = -EFAULT;
kfree_skb(skb); kfree_skb(skb);
goto error; goto error;
} else if (flags & MSG_SPLICE_PAGES) {
copy = 0;
} }
offset += copy; offset += copy;
...@@ -1791,6 +1796,10 @@ static int __ip6_append_data(struct sock *sk, ...@@ -1791,6 +1796,10 @@ static int __ip6_append_data(struct sock *sk,
} else if (flags & MSG_SPLICE_PAGES) { } else if (flags & MSG_SPLICE_PAGES) {
struct msghdr *msg = from; struct msghdr *msg = from;
err = -EIO;
if (WARN_ON_ONCE(copy > msg->msg_iter.count))
goto error;
err = skb_splice_from_iter(skb, &msg->msg_iter, copy, err = skb_splice_from_iter(skb, &msg->msg_iter, copy,
sk->sk_allocation); sk->sk_allocation);
if (err < 0) if (err < 0)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment