| Message ID | 20240726023359.879166-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: drop bad gso csum_start and offset in virtio_net_hdr | expand |
On Fri, Jul 26, 2024 at 4:34 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> ... > /* Kernel has a special handling for GSO_BY_FRAGS. */ > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > index 4b791e74529e1..9e49ffcc77071 100644 > --- a/net/ipv4/tcp_offload.c > +++ b/net/ipv4/tcp_offload.c > @@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, > if (thlen < sizeof(*th)) > goto out; > > + if (unlikely(skb->csum_start != skb->transport_header)) > + goto out; > + Using skb_transport_header() will make sure DEBUG_NET_WARN_ON_ONCE() will fire for debug kernels, with no additional costs for non debug kernels (compiler will generate not use skb->head at all) if (unlikely(skb->csum_start != skb_transport_header(skb) - skb->head)) goto out; (This will match the corresponding initialization in __tcp_v4_send_check())
On 7/26/24 04:32, Willem de Bruijn wrot> @@ -182,6 +171,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > if (gso_type != SKB_GSO_UDP_L4) > return -EINVAL; > break; > + case SKB_GSO_TCPV4: > + case SKB_GSO_TCPV6: I think we need to add here an additional check: if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) return -EINVAL; > + if (skb->csum_offset != offsetof(struct tcphdr, check)) > + return -EINVAL; > + break; > } > > /* Kernel has a special handling for GSO_BY_FRAGS. */ > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > index 4b791e74529e1..9e49ffcc77071 100644 > --- a/net/ipv4/tcp_offload.c > +++ b/net/ipv4/tcp_offload.c > @@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, > if (thlen < sizeof(*th)) > goto out; > > + if (unlikely(skb->csum_start != skb->transport_header)) > + goto out; Given that for packet injected from user-space, the transport offset is set to csum_start by skb_partial_csum_set(), do we need the above check? If so, why don't we need another similar one for csum_offset even here? Thanks, Paolo
On Fri, Jul 26, 2024 at 3:00 AM Eric Dumazet <edumazet@google.com> wrote: > > On Fri, Jul 26, 2024 at 4:34 AM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > From: Willem de Bruijn <willemb@google.com> > > ... > > > /* Kernel has a special handling for GSO_BY_FRAGS. */ > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > > index 4b791e74529e1..9e49ffcc77071 100644 > > --- a/net/ipv4/tcp_offload.c > > +++ b/net/ipv4/tcp_offload.c > > @@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, > > if (thlen < sizeof(*th)) > > goto out; > > > > + if (unlikely(skb->csum_start != skb->transport_header)) > > + goto out; > > + > > Using skb_transport_header() will make sure DEBUG_NET_WARN_ON_ONCE() > will fire for debug kernels, > with no additional costs for non debug kernels (compiler will generate > not use skb->head at all) > > if (unlikely(skb->csum_start != skb_transport_header(skb) - skb->head)) > goto out; > > (This will match the corresponding initialization in __tcp_v4_send_check()) Will do, thanks.
On Fri, Jul 26, 2024 at 4:23 AM Paolo Abeni <pabeni@redhat.com> wrote: > > On 7/26/24 04:32, Willem de Bruijn wrot> @@ -182,6 +171,11 @@ static > inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > if (gso_type != SKB_GSO_UDP_L4) > > return -EINVAL; > > break; > > + case SKB_GSO_TCPV4: > > + case SKB_GSO_TCPV6: > > I think we need to add here an additional check: > > if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) > return -EINVAL; > Historically this interface has been able to request VIRTIO_NET_HDR_GSO_* without VIRTIO_NET_HDR_F_NEEDS_CSUM. I agree that that makes little sense. Until now we have been accommodating it, however. See the else branch if that checksum offload flag is not set. I would love to clamp down on this, as those packets are essentially illegal. But we should probably leave that discussion for a separate patch? > > + if (skb->csum_offset != offsetof(struct tcphdr, check)) > > + return -EINVAL; > > + break; > > } > > > > /* Kernel has a special handling for GSO_BY_FRAGS. */ > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > > index 4b791e74529e1..9e49ffcc77071 100644 > > --- a/net/ipv4/tcp_offload.c > > +++ b/net/ipv4/tcp_offload.c > > @@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, > > if (thlen < sizeof(*th)) > > goto out; > > > > + if (unlikely(skb->csum_start != skb->transport_header)) > > + goto out; > > Given that for packet injected from user-space, the transport offset is > set to csum_start by skb_partial_csum_set(), do we need the above check? > If so, why don't we need another similar one for csum_offset even here? Same point. Sadly it is not set if checksum offload is not requested.
On 7/26/24 15:52, Willem de Bruijn wrote: > On Fri, Jul 26, 2024 at 4:23 AM Paolo Abeni <pabeni@redhat.com> wrote: >> >> On 7/26/24 04:32, Willem de Bruijn wrot> @@ -182,6 +171,11 @@ static >> inline int virtio_net_hdr_to_skb(struct sk_buff *skb, >>> if (gso_type != SKB_GSO_UDP_L4) >>> return -EINVAL; >>> break; >>> + case SKB_GSO_TCPV4: >>> + case SKB_GSO_TCPV6: >> >> I think we need to add here an additional check: >> >> if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) >> return -EINVAL; >> > > Historically this interface has been able to request > VIRTIO_NET_HDR_GSO_* without VIRTIO_NET_HDR_F_NEEDS_CSUM. I see. I looked at the SKB_GSO_UDP_L4 case, but I did not dig into history. > I would love to clamp down on this, as those packets are essentially > illegal. But we should probably leave that discussion for a separate > patch? Yep, I guess we have to keep the two discussion separate. As a consequence, I'm fine with the current checks (with Eric's suggested changes). Thanks, Paolo
Hello, This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf Thanks, Adrian.
On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > Hello, > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf Please provide a working backport, the change does not properly cherry-pick. greg k-h
Hello, My colleague Mathieu already submitted the tested patch here https://lore.kernel.org/stable/20240806122236.60183-1-mathieu.tortuyaux@gmail.com/T/#u. Links to Flatcar patch and test run: * https://github.com/flatcar/scripts/pull/2194/commits/33259937abe19f612faac255706d5a509666fbc9 * https://github.com/flatcar/scripts/actions/runs/10251425081 But this patch has been tested and submitted only for the 6.6.y branch. It will take some time to properly test the 6.1.y, as Flatcar is going to be fully upgraded on all channels to 6.6.y, but I will come back with the patch and test results. Thanks, Adrian.
On 24/08/07 04:12PM, Greg KH wrote: > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > Hello, > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > Please provide a working backport, the change does not properly > cherry-pick. > > greg k-h Hey Greg, hey Sasha, this patch also needs backporting to the 6.6.y and 6.10.y series as the buggy commit was backported to to all three series. I have tested against my local trees and it seems to apply cleanly on top of 6.6 and 6.10, yet if it helps I can also send out patches for stable versions of those, so we can have the fix for two out of three series while we wait for the backported version for 6.1. I also saw that the patch didn't make it to 6.10.4rc1 and is not in https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/queue-6.10 Cheers, Chris
On Wed, Aug 07, 2024 at 08:34:48PM +0200, Christian Heusel wrote: > On 24/08/07 04:12PM, Greg KH wrote: > > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > > Hello, > > > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > > > Please provide a working backport, the change does not properly > > cherry-pick. > > > > greg k-h > > Hey Greg, hey Sasha, > > this patch also needs backporting to the 6.6.y and 6.10.y series as the > buggy commit was backported to to all three series. What buggy commit? And how was this tested, it does not apply cleanly to the trees for me at all. confused, greg k-h
On 24/08/08 08:38AM, Greg KH wrote: > On Wed, Aug 07, 2024 at 08:34:48PM +0200, Christian Heusel wrote: > > On 24/08/07 04:12PM, Greg KH wrote: > > > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > > > Hello, > > > > > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > > > > > Please provide a working backport, the change does not properly > > > cherry-pick. > > > > > > greg k-h > > > > Hey Greg, hey Sasha, > > > > this patch also needs backporting to the 6.6.y and 6.10.y series as the > > buggy commit was backported to to all three series. > > What buggy commit? The issue is that commit e269d79c7d35 ("net: missing check virtio") introduces a bug which is fixed by 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") which it also carries a "Fixes:" tag for. Therefore it would be good to also get 89add40066f9 backported. > And how was this tested, it does not apply cleanly to the trees for me > at all. I have tested this with the procedure as described in [0]: $ git switch linux-6.10.y $ git cherry-pick -x 89add40066f9ed9abe5f7f886fe5789ff7e0c50e Auto-merging net/ipv4/udp_offload.c [linux-6.10.y fbc0d2bea065] net: drop bad gso csum_start and offset in virtio_net_hdr Author: Willem de Bruijn <willemb@google.com> Date: Mon Jul 29 16:10:12 2024 -0400 3 files changed, 12 insertions(+), 11 deletions(-) This also works for linux-6.6.y, but not for linux-6.1.y, as it fails with a merge error there. The relevant commit is confirmed to fix the issue in the relevant Githu issue here[1]: @marek22k commented > They both fix the problem for me. > confused, Sorry for the confusion! I hope the above clears things up a little :) > greg k-h Cheers, Christian [0]: https://lore.kernel.org/all/2024060624-platinum-ladies-9214@gregkh/ [1]: https://github.com/tailscale/tailscale/issues/13041#issuecomment-2272326491
Hello, Hopefully I can also offer some clarity on the issue in the context of the Linux kernel version 6.6.44. Regarding the 6.6.y case, this commit is the offending one: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.6.y&id=90d41ebe0cd4635f6410471efc1dd71b33e894cf With this commit, Flatcar virtual machines using Linux kernel version 6.6.44 running on QEMU-KVM AMD64 hosts started having these kind of messages in the dmesg while the network tests were failing: ``` [ 237.422038] eth0: bad gso: type: 1, size: 1432 ``` ```bash $ dmesg | grep "bad gso" | wc -l 531 ``` We observed that by cherry-picking this commit https://github.com/torvalds/linux/commit/89add40066f9ed9abe5f7f886fe5789ff7e0c50e on the 6.6.44 tree, the failures were fixed and patch has already been sent here: https://lore.kernel.org/stable/20240806122236.60183-1-mathieu.tortuyaux@gmail.com/T/#u To give some context, in the 89add40066f9ed9abe5f7f886fe5789ff7e0c50e commit description, it is stated that the commit fixes the https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8, that is how we got to testing the fix in the first place. Flatcar patch commit: https://github.com/flatcar/scripts/pull/2194/commits/33259937abe19f612faac255706d5a509666fbc9 that has been built for the Flatcar guests and successfully tested on QEMU-KVM AMD64 hosts. I tried manually to directly cherry-pick 89add40066f9ed9abe5f7f886fe5789ff7e0c50e on the 6.6.y branch and it works fine: ```bash git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git -b linux-6.6.y cd linux git remote add upstream https://github.com/torvalds/linux/ git fetch upstream git cherry-pick 89add40066f9ed9abe5f7f886fe5789ff7e0c50e ``` Related to the 6.1.y tree, the commit 89add40066f9ed9abe5f7f886fe5789ff7e0c50e cannot be cherry-picked without conflicts, and it requires more careful attention and testing. Related to the 6.10.y tree, the commit 89add40066f9ed9abe5f7f886fe5789ff7e0c50e can be cherry-picked, but has not been tested by us. Thanks, Adrian.
On 24/08/08 11:52AM, Christian Heusel wrote: > On 24/08/08 08:38AM, Greg KH wrote: > > On Wed, Aug 07, 2024 at 08:34:48PM +0200, Christian Heusel wrote: > > > On 24/08/07 04:12PM, Greg KH wrote: > > > > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > > > > Hello, > > > > > > > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > > > > > > > Please provide a working backport, the change does not properly > > > > cherry-pick. > > > > > > > > greg k-h > > > > > > Hey Greg, hey Sasha, > > > > > > this patch also needs backporting to the 6.6.y and 6.10.y series as the > > > buggy commit was backported to to all three series. > > > > What buggy commit? > > The issue is that commit e269d79c7d35 ("net: missing check virtio") > introduces a bug which is fixed by 89add40066f9 ("net: drop bad gso > csum_start and offset in virtio_net_hdr") which it also carries a > "Fixes:" tag for. > > Therefore it would be good to also get 89add40066f9 backported. > > > And how was this tested, it does not apply cleanly to the trees for me > > at all. > > I have tested this with the procedure as described in [0]: > > $ git switch linux-6.10.y > $ git cherry-pick -x 89add40066f9ed9abe5f7f886fe5789ff7e0c50e > Auto-merging net/ipv4/udp_offload.c > [linux-6.10.y fbc0d2bea065] net: drop bad gso csum_start and offset in virtio_net_hdr > Author: Willem de Bruijn <willemb@google.com> > Date: Mon Jul 29 16:10:12 2024 -0400 > 3 files changed, 12 insertions(+), 11 deletions(-) > > This also works for linux-6.6.y, but not for linux-6.1.y, as it fails > with a merge error there. > > The relevant commit is confirmed to fix the issue in the relevant Githu > issue here[1]: > > @marek22k commented > > They both fix the problem for me. > > > confused, > > Sorry for the confusion! I hope the above clears things up a little :) > > > greg k-h > > Cheers, > Christian > > [0]: https://lore.kernel.org/all/2024060624-platinum-ladies-9214@gregkh/ > [1]: https://github.com/tailscale/tailscale/issues/13041#issuecomment-2272326491 Since I didn't hear from anybody so far about the above issue it's a bit unclear on how to proceed here. I still think that I would make sense to go with my above suggestion about patching at least 2 out of the 3 stable series where the patch applies cleanly. ~ Chris
On Wed, Aug 14, 2024 at 11:46:30AM +0200, Christian Heusel wrote: > On 24/08/08 11:52AM, Christian Heusel wrote: > > On 24/08/08 08:38AM, Greg KH wrote: > > > On Wed, Aug 07, 2024 at 08:34:48PM +0200, Christian Heusel wrote: > > > > On 24/08/07 04:12PM, Greg KH wrote: > > > > > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > > > > > Hello, > > > > > > > > > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > > > > > > > > > Please provide a working backport, the change does not properly > > > > > cherry-pick. > > > > > > > > > > greg k-h > > > > > > > > Hey Greg, hey Sasha, > > > > > > > > this patch also needs backporting to the 6.6.y and 6.10.y series as the > > > > buggy commit was backported to to all three series. > > > > > > What buggy commit? > > > > The issue is that commit e269d79c7d35 ("net: missing check virtio") > > introduces a bug which is fixed by 89add40066f9 ("net: drop bad gso > > csum_start and offset in virtio_net_hdr") which it also carries a > > "Fixes:" tag for. > > > > Therefore it would be good to also get 89add40066f9 backported. > > > > > And how was this tested, it does not apply cleanly to the trees for me > > > at all. > > > > I have tested this with the procedure as described in [0]: > > > > $ git switch linux-6.10.y > > $ git cherry-pick -x 89add40066f9ed9abe5f7f886fe5789ff7e0c50e > > Auto-merging net/ipv4/udp_offload.c > > [linux-6.10.y fbc0d2bea065] net: drop bad gso csum_start and offset in virtio_net_hdr > > Author: Willem de Bruijn <willemb@google.com> > > Date: Mon Jul 29 16:10:12 2024 -0400 > > 3 files changed, 12 insertions(+), 11 deletions(-) > > > > This also works for linux-6.6.y, but not for linux-6.1.y, as it fails > > with a merge error there. > > > > The relevant commit is confirmed to fix the issue in the relevant Githu > > issue here[1]: > > > > @marek22k commented > > > They both fix the problem for me. > > > > > confused, > > > > Sorry for the confusion! I hope the above clears things up a little :) > > > > > greg k-h > > > > Cheers, > > Christian > > > > [0]: https://lore.kernel.org/all/2024060624-platinum-ladies-9214@gregkh/ > > [1]: https://github.com/tailscale/tailscale/issues/13041#issuecomment-2272326491 > > Since I didn't hear from anybody so far about the above issue it's a bit > unclear on how to proceed here. I still think that I would make sense to > go with my above suggestion about patching at least 2 out of the 3 > stable series where the patch applies cleanly. > > ~ Chris Do what Greg said: Please provide a working backport, the change does not properly cherry-pick. that means, post backported patches to stable, copy list.
On 24/08/14 05:54AM, Michael S. Tsirkin wrote: > On Wed, Aug 14, 2024 at 11:46:30AM +0200, Christian Heusel wrote: > > On 24/08/08 11:52AM, Christian Heusel wrote: > > > On 24/08/08 08:38AM, Greg KH wrote: > > > > On Wed, Aug 07, 2024 at 08:34:48PM +0200, Christian Heusel wrote: > > > > > On 24/08/07 04:12PM, Greg KH wrote: > > > > > > On Mon, Aug 05, 2024 at 09:28:29PM +0000, avladu@cloudbasesolutions.com wrote: > > > > > > > Hello, > > > > > > > > > > > > > > This patch needs to be backported to the stable 6.1.x and 6.64.x branches, as the initial patch https://github.com/torvalds/linux/commit/e269d79c7d35aa3808b1f3c1737d63dab504ddc8 was backported a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.1.103&id=3D5b1997487a3f3373b0f580c8a20b56c1b64b0775 > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/virtio_net.h?h=3Dv6.6.44&id=3D90d41ebe0cd4635f6410471efc1dd71b33e894cf > > > > > > > > > > > > Please provide a working backport, the change does not properly > > > > > > cherry-pick. > > > > > > > > > > > > greg k-h > > > > > > > > > > Hey Greg, hey Sasha, > > > > > > > > > > this patch also needs backporting to the 6.6.y and 6.10.y series as the > > > > > buggy commit was backported to to all three series. > > > > > > > > What buggy commit? > > > > > > The issue is that commit e269d79c7d35 ("net: missing check virtio") > > > introduces a bug which is fixed by 89add40066f9 ("net: drop bad gso > > > csum_start and offset in virtio_net_hdr") which it also carries a > > > "Fixes:" tag for. > > > > > > Therefore it would be good to also get 89add40066f9 backported. > > > > > > > And how was this tested, it does not apply cleanly to the trees for me > > > > at all. > > > > > > I have tested this with the procedure as described in [0]: > > > > > > $ git switch linux-6.10.y > > > $ git cherry-pick -x 89add40066f9ed9abe5f7f886fe5789ff7e0c50e > > > Auto-merging net/ipv4/udp_offload.c > > > [linux-6.10.y fbc0d2bea065] net: drop bad gso csum_start and offset in virtio_net_hdr > > > Author: Willem de Bruijn <willemb@google.com> > > > Date: Mon Jul 29 16:10:12 2024 -0400 > > > 3 files changed, 12 insertions(+), 11 deletions(-) > > > > > > This also works for linux-6.6.y, but not for linux-6.1.y, as it fails > > > with a merge error there. > > > > > > The relevant commit is confirmed to fix the issue in the relevant Githu > > > issue here[1]: > > > > > > @marek22k commented > > > > They both fix the problem for me. > > > > > > > confused, > > > > > > Sorry for the confusion! I hope the above clears things up a little :) > > > > > > > greg k-h > > > > > > Cheers, > > > Christian > > > > > > [0]: https://lore.kernel.org/all/2024060624-platinum-ladies-9214@gregkh/ > > > [1]: https://github.com/tailscale/tailscale/issues/13041#issuecomment-2272326491 > > > > Since I didn't hear from anybody so far about the above issue it's a bit > > unclear on how to proceed here. I still think that I would make sense to > > go with my above suggestion about patching at least 2 out of the 3 > > stable series where the patch applies cleanly. > > > > ~ Chris > > > > Do what Greg said: > > Please provide a working backport, the change does not properly > cherry-pick. > > that means, post backported patches to stable, copy list. Alright, will do!
Hello, The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. Thanks, Adrian.
On 24/08/14 10:10AM, Adrian Vladu wrote: > Hello, > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > Thanks, Adrian. Yeah it's also queued up for 6.10, which I both missed (sorry for that!). If I'm able to properly backport the patch for 6.1 I'll send that one, but my hopes are not too high that this will work .. Have a nice day everybody :) Cheers, Chris
Christian Heusel wrote: > On 24/08/14 10:10AM, Adrian Vladu wrote: > > Hello, > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > Thanks, Adrian. > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > If I'm able to properly backport the patch for 6.1 I'll send that one, > but my hopes are not too high that this will work .. There are two conflicts. The one in include/linux/virtio_net.h is resolved by first backporting commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") We did not backport that to stable because there was some slight risk that applications might be affected. This has not surfaced. The conflict in net/ipv4/udp_offload.c is not so easy to address. There were lots of patches between v6.1 and linus/master, with far fewer of these betwee v6.1 and linux-stable/linux-6.1.y. We can also avoid the backport of fc8b2a6194693 and construct a custom version for this older kernel. All this is needed in virtio_net.h is + case SKB_GSO_UDP_L4: + case SKB_GSO_TCPV4: + case SKB_GSO_TCPV6: + if (skb->csum_offset != offsetof(struct tcphdr, check)) and in __udp_gso_segment + if (unlikely(skb_checksum_start(gso_skb) != + skb_transport_header(gso_skb))) + return ERR_PTR(-EINVAL); + The Fixes tag points to a commit introduced in 6.1. 6.6 is queued up, so this is the only release for which we have to create a custom patch, right? Let me know if you will send this, or I should?
Willem, On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > Christian Heusel wrote: > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > Hello, > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > Thanks, Adrian. > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > but my hopes are not too high that this will work .. > > There are two conflicts. > > The one in include/linux/virtio_net.h is resolved by first backporting > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > validation") > > We did not backport that to stable because there was some slight risk > that applications might be affected. This has not surfaced. > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > There were lots of patches between v6.1 and linus/master, with far > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in ALT, and there is no reported problems as of yet. 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ Thanks, > > We can also avoid the backport of fc8b2a6194693 and construct a custom > version for this older kernel. All this is needed in virtio_net.h is > > + case SKB_GSO_UDP_L4: > + case SKB_GSO_TCPV4: > + case SKB_GSO_TCPV6: > + if (skb->csum_offset != offsetof(struct tcphdr, check)) > > and in __udp_gso_segment > > + if (unlikely(skb_checksum_start(gso_skb) != > + skb_transport_header(gso_skb))) > + return ERR_PTR(-EINVAL); > + > > The Fixes tag points to a commit introduced in 6.1. 6.6 is queued up, > so this is the only release for which we have to create a custom > patch, right? > > Let me know if you will send this, or I should? >
Vitaly Chikunov wrote: > Willem, > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > Christian Heusel wrote: > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > Hello, > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > Thanks, Adrian. > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > but my hopes are not too high that this will work .. > > > > There are two conflicts. > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > validation") > > > > We did not backport that to stable because there was some slight risk > > that applications might be affected. This has not surfaced. > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > There were lots of patches between v6.1 and linus/master, with far > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > ALT, and there is no reported problems as of yet. > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ That's good to hear. These are all fine to go to 6.1 stable.
Hi, On Wed, Aug 21, 2024 at 10:05:12AM -0400, Willem de Bruijn wrote: > Vitaly Chikunov wrote: > > Willem, > > > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > > Christian Heusel wrote: > > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > > Hello, > > > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > > > Thanks, Adrian. > > > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > > but my hopes are not too high that this will work .. > > > > > > There are two conflicts. > > > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > > validation") > > > > > > We did not backport that to stable because there was some slight risk > > > that applications might be affected. This has not surfaced. > > > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > > There were lots of patches between v6.1 and linus/master, with far > > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > > ALT, and there is no reported problems as of yet. > > > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ > > That's good to hear. > > These are all fine to go to 6.1 stable. FWIW, as we are hit by this issue for Debian bookworm, we have testing as well from David Prévot <taffit@debian.org>, cf. the report in https://bugs.debian.org/1079684 . He mentions that the 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") patch does not apply cleanly, looks to be because of 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") from 6.2-rc1, which are reverted in the commit. Regards, Salvatore
Hi, On Mon, Aug 26, 2024 at 04:10:21PM +0200, Salvatore Bonaccorso wrote: > Hi, > > On Wed, Aug 21, 2024 at 10:05:12AM -0400, Willem de Bruijn wrote: > > Vitaly Chikunov wrote: > > > Willem, > > > > > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > > > Christian Heusel wrote: > > > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > > > Hello, > > > > > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > > > > > Thanks, Adrian. > > > > > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > > > but my hopes are not too high that this will work .. > > > > > > > > There are two conflicts. > > > > > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > > > validation") > > > > > > > > We did not backport that to stable because there was some slight risk > > > > that applications might be affected. This has not surfaced. > > > > > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > > > There were lots of patches between v6.1 and linus/master, with far > > > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > > > > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > > > ALT, and there is no reported problems as of yet. > > > > > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > > > > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ > > > > That's good to hear. > > > > These are all fine to go to 6.1 stable. > > FWIW, as we are hit by this issue for Debian bookworm, we have testing > as well from David Prévot <taffit@debian.org>, cf. the report in > https://bugs.debian.org/1079684 . > > He mentions that the 9840036786d9 ("gso: fix dodgy bit handling for > GSO_UDP_L4") patch does not apply cleanly, looks to be because of > 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") > from 6.2-rc1, which are reverted in the commit. Just to give an additional confirmation: Applying 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") addresses the issue from https://bugs.debian.org/1079684 matching https://bugzilla.kernel.org/show_bug.cgi?id=219129 I tested it with the iperf3 based reproducers. Tested-by: Salvatore Bonaccorso <carnil@debian.org> Regards, Salvatore
On Wed, Aug 21, 2024 at 10:05:12AM -0400, Willem de Bruijn wrote: > Vitaly Chikunov wrote: > > Willem, > > > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > > Christian Heusel wrote: > > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > > Hello, > > > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > > > Thanks, Adrian. > > > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > > but my hopes are not too high that this will work .. > > > > > > There are two conflicts. > > > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > > validation") > > > > > > We did not backport that to stable because there was some slight risk > > > that applications might be affected. This has not surfaced. > > > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > > There were lots of patches between v6.1 and linus/master, with far > > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > > ALT, and there is no reported problems as of yet. > > > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ > > That's good to hear. > > These are all fine to go to 6.1 stable. Can someone please send a series of backported patches to us so that we know exactly what to apply and in what order and why they need to be applied at all? thanks, greg k-h
On Mon, Aug 26, 2024 at 10:07:50PM +0200, Salvatore Bonaccorso wrote: > Hi, > > On Mon, Aug 26, 2024 at 04:10:21PM +0200, Salvatore Bonaccorso wrote: > > Hi, > > > > On Wed, Aug 21, 2024 at 10:05:12AM -0400, Willem de Bruijn wrote: > > > Vitaly Chikunov wrote: > > > > Willem, > > > > > > > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > > > > Christian Heusel wrote: > > > > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > > > > Hello, > > > > > > > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > > > > > > > Thanks, Adrian. > > > > > > > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > > > > but my hopes are not too high that this will work .. > > > > > > > > > > There are two conflicts. > > > > > > > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > > > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > > > > validation") > > > > > > > > > > We did not backport that to stable because there was some slight risk > > > > > that applications might be affected. This has not surfaced. > > > > > > > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > > > > There were lots of patches between v6.1 and linus/master, with far > > > > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > > > > > > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > > > > ALT, and there is no reported problems as of yet. > > > > > > > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > > > > > > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ > > > > > > That's good to hear. > > > > > > These are all fine to go to 6.1 stable. > > > > FWIW, as we are hit by this issue for Debian bookworm, we have testing > > as well from David Prévot <taffit@debian.org>, cf. the report in > > https://bugs.debian.org/1079684 . > > > > He mentions that the 9840036786d9 ("gso: fix dodgy bit handling for > > GSO_UDP_L4") patch does not apply cleanly, looks to be because of > > 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") > > from 6.2-rc1, which are reverted in the commit. > > Just to give an additional confirmation: Applying > > 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") Ah, that works, thanks! greg k-h
On Tue, Aug 27, 2024 at 03:16:50PM +0200, Greg KH wrote: > On Mon, Aug 26, 2024 at 10:07:50PM +0200, Salvatore Bonaccorso wrote: > > Hi, > > > > On Mon, Aug 26, 2024 at 04:10:21PM +0200, Salvatore Bonaccorso wrote: > > > Hi, > > > > > > On Wed, Aug 21, 2024 at 10:05:12AM -0400, Willem de Bruijn wrote: > > > > Vitaly Chikunov wrote: > > > > > Willem, > > > > > > > > > > On Wed, Aug 14, 2024 at 09:53:58AM GMT, Willem de Bruijn wrote: > > > > > > Christian Heusel wrote: > > > > > > > On 24/08/14 10:10AM, Adrian Vladu wrote: > > > > > > > > Hello, > > > > > > > > > > > > > > > > The 6.6.y branch has the patch already in the stable queue -> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=3e713b73c01fac163a5c8cb0953d1e300407a773, and it should be available in the 6.6.46 upcoming minor. > > > > > > > > > > > > > > > > Thanks, Adrian. > > > > > > > > > > > > > > Yeah it's also queued up for 6.10, which I both missed (sorry for that!). > > > > > > > If I'm able to properly backport the patch for 6.1 I'll send that one, > > > > > > > but my hopes are not too high that this will work .. > > > > > > > > > > > > There are two conflicts. > > > > > > > > > > > > The one in include/linux/virtio_net.h is resolved by first backporting > > > > > > commit fc8b2a6194693 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 > > > > > > validation") > > > > > > > > > > > > We did not backport that to stable because there was some slight risk > > > > > > that applications might be affected. This has not surfaced. > > > > > > > > > > > > The conflict in net/ipv4/udp_offload.c is not so easy to address. > > > > > > There were lots of patches between v6.1 and linus/master, with far > > > > > > fewer of these betwee v6.1 and linux-stable/linux-6.1.y. > > > > > > > > > > BTW, we successfully cherry-picked 3 suggested[1] commits over v6.1.105 in > > > > > ALT, and there is no reported problems as of yet. > > > > > > > > > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > > > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > > > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > > > > > > > > > [1] https://lore.kernel.org/all/2024081147-altitude-luminous-19d1@gregkh/ > > > > > > > > That's good to hear. > > > > > > > > These are all fine to go to 6.1 stable. > > > > > > FWIW, as we are hit by this issue for Debian bookworm, we have testing > > > as well from David Prévot <taffit@debian.org>, cf. the report in > > > https://bugs.debian.org/1079684 . > > > > > > He mentions that the 9840036786d9 ("gso: fix dodgy bit handling for > > > GSO_UDP_L4") patch does not apply cleanly, looks to be because of > > > 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") > > > from 6.2-rc1, which are reverted in the commit. > > > > Just to give an additional confirmation: Applying > > > > 1fd54773c267 ("udp: allow header check for dodgy GSO_UDP_L4 packets.") Interestingly, I don't need this commit cherry-picked when applying above patchset over v6.1.106 (with my git 2.42.2). It applies cleanly with two "Auto-merging" messages, then 2nd and 3rd hunks are not applied. It seems that 1fd54773c267 only adds the changes that following 9840036786d9 removes (in the 2nd and 3rd hunks). And the git is smart enough to figure that out and just don't apply them when cherry-picking. That explains why some commits that I say is apply cleanly some other people cannot apply. Thanks, > > 9840036786d9 ("gso: fix dodgy bit handling for GSO_UDP_L4") > > fc8b2a619469 ("net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation") > > 89add40066f9 ("net: drop bad gso csum_start and offset in virtio_net_hdr") > > Ah, that works, thanks! > > greg k-h
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index d1d7825318c32..6c395a2600e8d 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -56,7 +56,6 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, unsigned int thlen = 0; unsigned int p_off = 0; unsigned int ip_proto; - u64 ret, remainder, gso_size; if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) { @@ -99,16 +98,6 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); - if (hdr->gso_size) { - gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size); - ret = div64_u64_rem(skb->len, gso_size, &remainder); - if (!(ret && (hdr->gso_size > needed) && - ((remainder > needed) || (remainder == 0)))) { - return -EINVAL; - } - skb_shinfo(skb)->tx_flags |= SKBFL_SHARED_FRAG; - } - if (!pskb_may_pull(skb, needed)) return -EINVAL; @@ -182,6 +171,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, if (gso_type != SKB_GSO_UDP_L4) return -EINVAL; break; + case SKB_GSO_TCPV4: + case SKB_GSO_TCPV6: + if (skb->csum_offset != offsetof(struct tcphdr, check)) + return -EINVAL; + break; } /* Kernel has a special handling for GSO_BY_FRAGS. */ diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index 4b791e74529e1..9e49ffcc77071 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c @@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, if (thlen < sizeof(*th)) goto out; + if (unlikely(skb->csum_start != skb->transport_header)) + goto out; + if (!pskb_may_pull(skb, thlen)) goto out; diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index aa2e0a28ca613..f521152c40871 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -278,6 +278,9 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, if (gso_skb->len <= sizeof(*uh) + mss) return ERR_PTR(-EINVAL); + if (unlikely(gso_skb->csum_start != gso_skb->transport_header)) + return ERR_PTR(-EINVAL); + if (skb_gso_ok(gso_skb, features | NETIF_F_GSO_ROBUST)) { /* Packet is from an untrusted source, reset gso_segs. */ skb_shinfo(gso_skb)->gso_segs = DIV_ROUND_UP(gso_skb->len - sizeof(*uh),