[PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[shiming.cheng]

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;
+	}
+
 	if (!pskb_may_pull(skb, write_len))
 		return -ENOMEM;
 
-- 
2.18.0

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Willem de Bruijn]

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
> 

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Jakub Kicinski]

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.

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Lena Wang (王娜)]

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

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Jakub Kicinski]

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.

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Lena Wang (王娜)]

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

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Willem de Bruijn]

> 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

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Willem de Bruijn]

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

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Lena Wang (王娜)]

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

Re: [PATCH net] net: prevent pulling SKB_GSO_FRAGLIST skb

[Willem de Bruijn]

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.