| Message ID | 20240428142913.18666-1-shiming.cheng@mediatek.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: prevent pulling SKB_GSO_FRAGLIST skb | expand |
shiming.cheng@ wrote: > From: Shiming Cheng <shiming.cheng@mediatek.com> > > BPF or TC callers may pull in a length longer than skb_headlen() > for a SKB_GSO_FRAGLIST skb. The data in fraglist will be pulled > into the linear space. However it destroys the skb's structure > and may result in an invalid segmentation or kernel exception. > > So we should add protection to stop the operation and return > error to remind callers. > > Fixes: 3a1296a38d0c ("net: Support GRO/GSO fraglist chaining.") > Signed-off-by: Shiming Cheng <shiming.cheng@mediatek.com> > Signed-off-by: Lena Wang <lena.wang@mediatek.com> > > --- > net/core/skbuff.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index f68f2679b086..2d35e009e814 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -6100,6 +6100,12 @@ EXPORT_SYMBOL(skb_vlan_untag); > > int skb_ensure_writable(struct sk_buff *skb, unsigned int write_len) > { > + if (skb_is_gso(skb) && > + (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && > + write_len > skb_headlen(skb)) { > + return -ENOMEM; > + } > + Most callers of skb_ensure_writable pull less than headlen. It might be good to start with the write_len check. Before looking at gso type. > if (!pskb_may_pull(skb, write_len)) > return -ENOMEM; > > -- > 2.18.0 >
On Sun, 28 Apr 2024 22:29:13 +0800 shiming.cheng@mediatek.com wrote: > From: Shiming Cheng <shiming.cheng@mediatek.com> > > BPF or TC callers may pull in a length longer than skb_headlen() > for a SKB_GSO_FRAGLIST skb. The data in fraglist will be pulled > into the linear space. However it destroys the skb's structure > and may result in an invalid segmentation or kernel exception. > > So we should add protection to stop the operation and return > error to remind callers. One of the fixes you posted breaks the tools/testing/selftests/net/udpgro_fwd.sh selftest. Please investigate, and either adjust the test or the fix.
On Mon, 2024-04-29 at 06:42 -0700, Jakub Kicinski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On Sun, 28 Apr 2024 22:29:13 +0800 shiming.cheng@mediatek.com wrote: > > From: Shiming Cheng <shiming.cheng@mediatek.com> > > > > BPF or TC callers may pull in a length longer than skb_headlen() > > for a SKB_GSO_FRAGLIST skb. The data in fraglist will be pulled > > into the linear space. However it destroys the skb's structure > > and may result in an invalid segmentation or kernel exception. > > > > So we should add protection to stop the operation and return > > error to remind callers. > > One of the fixes you posted breaks the > > tools/testing/selftests/net/udpgro_fwd.sh > > selftest. Please investigate, and either adjust the test or the fix. Dear Jakub, Sorry for late response. As we do not make selftest before, I try to build a test environmen and cost time to apply sudo access right in our company server. Now it blocks to generate xdp_dummy.bpf.o. Could you please give some guidline about the script test step? Thanks. Could you give more info about the failed situation? Is it this fix "[PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb" failed? Which case is failed? Is it possible that the test case has issue? > { > > +if (skb_is_gso(skb) && > > + (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && > > + write_len > skb_headlen(skb)) { > > +return -ENOMEM; > > +} > > + > > Most callers of skb_ensure_writable pull less than headlen. > It might be good to start with the write_len check. Before > looking at gso type. > Dear Willem, I will udpate as your advice in v2 as: +if (write_len > skb_headlen(skb) && skb_is_gso(skb) && + (skb_shinfo (skb)->gso_type & SKB_GSO_FRAGLIST)) { About selftests/net/udpgro_fwd.sh case failed, do you know the reason or have any advice? Thanks Lena
On Wed, 15 May 2024 09:02:35 +0000 Lena Wang (王娜) wrote: > > One of the fixes you posted breaks the > > > > tools/testing/selftests/net/udpgro_fwd.sh > > > > selftest. Please investigate, and either adjust the test or the fix. > > Dear Jakub, > Sorry for late response. > As we do not make selftest before, I try to build a test environmen and > cost time to apply sudo access right in our company server. Please read: https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style Depending on your setup sudo may not be needed. > Now it blocks to generate xdp_dummy.bpf.o. Could you please give some guidline > about the script test step? Thanks. Install clang and run make? Please share some outputs or more details, I'm not sure what the problem is > Could you give more info about the failed situation? > Is it this fix "[PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb" > failed? > Which case is failed? These are the results, as far as I can tell: https://netdev-3.bots.linux.dev/vmksft-net/results/573200/24-udpgro-fwd-sh/ > Is it possible that the test case has issue? Entirely possible, yes.
On Thu, 2024-05-16 at 08:11 -0700, Jakub Kicinski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On Wed, 15 May 2024 09:02:35 +0000 Lena Wang (王娜) wrote: > > > One of the fixes you posted breaks the > > > > > > tools/testing/selftests/net/udpgro_fwd.sh > > > > > > selftest. Please investigate, and either adjust the test or the > fix. > > > > Dear Jakub, > > Sorry for late response. > > As we do not make selftest before, I try to build a test environmen > and > > cost time to apply sudo access right in our company server. > > Please read: > https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style > > Depending on your setup sudo may not be needed. > > > Now it blocks to generate xdp_dummy.bpf.o. Could you please give > some guidline > > about the script test step? Thanks. > > Install clang and run make? Please share some outputs or more > details, > I'm not sure what the problem is > > > Could you give more info about the failed situation? > > Is it this fix "[PATCH net] net: prevent pulling SKB_GSO_FRAGLIST > skb" > > failed? > > Which case is failed? > > These are the results, as far as I can tell: > > https://netdev-3.bots.linux.dev/vmksft-net/results/573200/24-udpgro-fwd-sh/ > > > Is it possible that the test case has issue? > > Entirely possible, yes. Dear Jakub, Thanks your guidence. I have set up the test environment and part of test cases could pass. The problem now is the ethtool in ubuntu can't support "rx-gro-list" and "rx-udp-gro-forwarding" although it is updated to version 6.7 from https://mirrors.edge.kernel.org/pub/software/network/ethtool. There is another verison in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool. We download the sourcecode but don't know how to compile for ubuntu as no ./configure there. Is it the one we should use? If yes, could you please show me how to compile and install this ethtool? Below are my test result with current ethtool now: IPv4 No GRO ok ethtool: bad command line argument(s) For more information run ethtool -h GRO frag list fail - received 10 packets, expected 1 ./udpgro_fwd.sh: line 231: rx-udp-gro-forwarding: command not found GRO fwd fail - received 10 packets, expected 1 UDP fwd perf udp rx: 77 MB/s 63024 calls/s udp tx: 2099 MB/s 35603 calls/s 35603 msg/s udp rx: 89 MB/s 73011 calls/s udp tx: 2090 MB/s 35459 calls/s 35459 msg/s udp rx: 82 MB/s 66799 calls/s udp tx: 2100 MB/s 35621 calls/s 35621 msg/s ./udpgro_fwd.sh: line 239: rx-udp-gro-forwarding: command not found UDP GRO fwd perf udp rx: 47 MB/s 39052 calls/s udp tx: 1654 MB/s 28059 calls/s 28059 msg/s udp rx: 71 MB/s 58102 calls/s udp tx: 1932 MB/s 32774 calls/s 32774 msg/s udp rx: 85 MB/s 69976 calls/s udp tx: 2094 MB/s 35524 calls/s 35524 msg/s ./udpgro_fwd.sh: line 244: rx-gro-list: command not found GRO frag list over UDP tunnel fail - received 10 packets, expected 1 ./udpgro_fwd.sh: line 251: rx-udp-gro-forwarding: command not found GRO fwd over UDP tunnel fail - received 10 packets, expected 1 UDP tunnel fwd perf udp rx: 45 MB/s 37075 calls/s udp tx: 1070 MB/s 18149 calls/s 18149 msg/s udp rx: 55 MB/s 45396 calls/s udp tx: 1070 MB/s 18159 calls/s 18159 msg/s udp rx: 55 MB/s 45032 calls/s udp tx: 1075 MB/s 18233 calls/s 18233 msg/s ./udpgro_fwd.sh: line 263: rx-udp-gro-forwarding: command not found UDP tunnel GRO fwd perf udp rx: 45 MB/s 37440 calls/s udp tx: 1065 MB/s 18078 calls/s 18078 msg/s udp rx: 70 MB/s 57512 calls/s udp tx: 1066 MB/s 18091 calls/s 18091 msg/s udp rx: 68 MB/s 55432 calls/s udp tx: 1067 MB/s 18110 calls/s 18110 msg/s Thanks Lena
> The problem now is the ethtool in ubuntu can't support "rx-gro-list" > and "rx-udp-gro-forwarding" although it is updated to version 6.7 from > https://mirrors.edge.kernel.org/pub/software/network/ethtool. > > There is another verison in > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool. > We download the sourcecode but don't know how to compile for ubuntu as > no ./configure there. > > Is it the one we should use? If yes, could you please show me how to > compile and install this ethtool? https://git.kernel.org/pub/scm/network/ethtool/ethtool.git is the upstream ethtool repo. Since you are testing a custom built kernel, there are other hacky ways to configure a feature if you lack a userspace component: - just hardcode on or off and reboot - use YNL ethtool (but features is not implemented yet?) - write your own netlink helper - abuse some existing kernel API to toggle it, like a rarely uses systl
Willem de Bruijn wrote: > > The problem now is the ethtool in ubuntu can't support "rx-gro-list" > > and "rx-udp-gro-forwarding" although it is updated to version 6.7 from > > https://mirrors.edge.kernel.org/pub/software/network/ethtool. > > > > There is another verison in > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool. > > We download the sourcecode but don't know how to compile for ubuntu as > > no ./configure there. > > > > Is it the one we should use? If yes, could you please show me how to > > compile and install this ethtool? > > https://git.kernel.org/pub/scm/network/ethtool/ethtool.git is the > upstream ethtool repo. > > Since you are testing a custom built kernel, there are other hacky > ways to configure a feature if you lack a userspace component: > > - just hardcode on or off and reboot > - use YNL ethtool (but features is not implemented yet?) > - write your own netlink helper > - abuse some existing kernel API to toggle it, like a rarely uses systl And as shared off-line, virtme-ng (vng) can be a good option for working on tools/testing/selftests too. Ideally ``` vng -v -b -f tools/testing/selftests/net make headers make -C tools/testing/selftests/net vng -v -r arch/x86/boot/bzImage --user root # inside the VM make -C tools/testing/selftests TARGETS=net run_tests ``` Though last time I tried I had to use a slightly more roundabout ``` make defconfig; make kvm_guest.config ./scripts/kconfig/merge_config.sh -m .config tools/testing/selftests/net/config make olddefconfig make -j $(nproc) bzImage make headers make -C tools/testing/selftests/net vng -v -r arch/x86/boot/bzImage --user root ``` https://lpc.events/event/17/contributions/1506/attachments/1143/2441/virtme-ng.pdf
On Thu, 2024-05-23 at 10:59 -0400, Willem de Bruijn wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Willem de Bruijn wrote: > > > The problem now is the ethtool in ubuntu can't support "rx-gro- > list" > > > and "rx-udp-gro-forwarding" although it is updated to version 6.7 > from > > > https://mirrors.edge.kernel.org/pub/software/network/ethtool. > > > > > > There is another verison in > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool > . > > > We download the sourcecode but don't know how to compile for > ubuntu as > > > no ./configure there. > > > > > > Is it the one we should use? If yes, could you please show me > how to > > > compile and install this ethtool? > > > > https://git.kernel.org/pub/scm/network/ethtool/ethtool.git is the > > upstream ethtool repo. > > > > Since you are testing a custom built kernel, there are other hacky > > ways to configure a feature if you lack a userspace component: > > > > - just hardcode on or off and reboot > > - use YNL ethtool (but features is not implemented yet?) > > - write your own netlink helper > > - abuse some existing kernel API to toggle it, like a rarely uses > systl > > And as shared off-line, virtme-ng (vng) can be a good option for > working on tools/testing/selftests too. > > Ideally > > ``` > vng -v -b -f tools/testing/selftests/net > make headers > make -C tools/testing/selftests/net > > vng -v -r arch/x86/boot/bzImage --user root > # inside the VM > make -C tools/testing/selftests TARGETS=net run_tests > ``` > > Though last time I tried I had to use a slightly more roundabout > > ``` > make defconfig; make kvm_guest.config > ./scripts/kconfig/merge_config.sh -m .config > tools/testing/selftests/net/config > make olddefconfig > make -j $(nproc) bzImage > make headers > make -C tools/testing/selftests/net > > vng -v -r arch/x86/boot/bzImage --user root > ``` > > > https://lpc.events/event/17/contributions/1506/attachments/1143/2441/virtme-ng.pdf Dear Willem, In https://github.com/arighi/virtme-ng it needs kernel 6.5 to setup. Current our enviroument doesn't support and we prepare to install a PC with a new ubuntu22.04. Do you know any request for ubuntu version to run vng, Which version is more fit for? Thanks Lena
Lena Wang (王娜) wrote: > On Thu, 2024-05-23 at 10:59 -0400, Willem de Bruijn wrote: > > > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > Willem de Bruijn wrote: > > > > The problem now is the ethtool in ubuntu can't support "rx-gro- > > list" > > > > and "rx-udp-gro-forwarding" although it is updated to version 6.7 > > from > > > > https://mirrors.edge.kernel.org/pub/software/network/ethtool. > > > > > > > > There is another verison in > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool > > . > > > > We download the sourcecode but don't know how to compile for > > ubuntu as > > > > no ./configure there. > > > > > > > > Is it the one we should use? If yes, could you please show me > > how to > > > > compile and install this ethtool? > > > > > > https://git.kernel.org/pub/scm/network/ethtool/ethtool.git is the > > > upstream ethtool repo. > > > > > > Since you are testing a custom built kernel, there are other hacky > > > ways to configure a feature if you lack a userspace component: > > > > > > - just hardcode on or off and reboot > > > - use YNL ethtool (but features is not implemented yet?) > > > - write your own netlink helper > > > - abuse some existing kernel API to toggle it, like a rarely uses > > systl > > > > And as shared off-line, virtme-ng (vng) can be a good option for > > working on tools/testing/selftests too. > > > > Ideally > > > > ``` > > vng -v -b -f tools/testing/selftests/net > > make headers > > make -C tools/testing/selftests/net > > > > vng -v -r arch/x86/boot/bzImage --user root > > # inside the VM > > make -C tools/testing/selftests TARGETS=net run_tests > > ``` > > > > Though last time I tried I had to use a slightly more roundabout > > > > ``` > > make defconfig; make kvm_guest.config > > ./scripts/kconfig/merge_config.sh -m .config > > tools/testing/selftests/net/config > > make olddefconfig > > make -j $(nproc) bzImage > > make headers > > make -C tools/testing/selftests/net > > > > vng -v -r arch/x86/boot/bzImage --user root > > ``` > > > > > > > https://lpc.events/event/17/contributions/1506/attachments/1143/2441/virtme-ng.pdf > > Dear Willem, > In https://github.com/arighi/virtme-ng it needs kernel 6.5 to setup. > Current our enviroument doesn't support and we prepare to install a PC > with a new ubuntu22.04. > > Do you know any request for ubuntu version to run vng, Which version is > more fit for? Let's take these configuration questions offline. I've responded.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f68f2679b086..2d35e009e814 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -6100,6 +6100,12 @@ EXPORT_SYMBOL(skb_vlan_untag); int skb_ensure_writable(struct sk_buff *skb, unsigned int write_len) { + if (skb_is_gso(skb) && + (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && + write_len > skb_headlen(skb)) { + return -ENOMEM; + } + if (!pskb_may_pull(skb, write_len)) return -ENOMEM;