[PATCH 1/1] drivers:net: fix return value check in be_lancer_xmit_workarounds

[PATCH 1/1] drivers:net: fix return value check in be_lancer_xmit_workarounds

[Yuanjun Gong]

in be_lancer_xmit_workarounds, it should go to label 'err' if
an unexpected value is returned by pskb_trim.

Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 18c2fc880d09..eba29a2e0e8b 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
 	    (lancer_chip(adapter) || BE3_chip(adapter) ||
 	     skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) {
 		ip = (struct iphdr *)ip_hdr(skb);
-		pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len));
+		if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len))))
+			goto err;
 	}
 
 	/* If vlan tag is already inlined in the packet, skip HW VLAN
-- 
2.17.1

Re: [PATCH 1/1] drivers:net: fix return value check in be_lancer_xmit_workarounds

[Kuniyuki Iwashima]

From: Yuanjun Gong <ruc_gongyuanjun@163.com>
Date: Mon, 17 Jul 2023 22:45:32 +0800
> in be_lancer_xmit_workarounds, it should go to label 'err' if
> an unexpected value is returned by pskb_trim.
>

Fixes tag needed here.

> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 18c2fc880d09..eba29a2e0e8b 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
>  	    (lancer_chip(adapter) || BE3_chip(adapter) ||
>  	     skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) {
>  		ip = (struct iphdr *)ip_hdr(skb);
> -		pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len));
> +		if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len))))
> +			goto err;

This should be `goto tx_drop`, or we'll leak skb.


>  	}
>  
>  	/* If vlan tag is already inlined in the packet, skip HW VLAN
> -- 
> 2.17.1

Re: [PATCH 1/1] drivers:net: fix return value check in be_lancer_xmit_workarounds

[Simon Horman]

On Mon, Jul 17, 2023 at 12:32:59PM -0700, Kuniyuki Iwashima wrote:
> From: Yuanjun Gong <ruc_gongyuanjun@163.com>
> Date: Mon, 17 Jul 2023 22:45:32 +0800
> > in be_lancer_xmit_workarounds, it should go to label 'err' if
> > an unexpected value is returned by pskb_trim.
> >
> 
> Fixes tag needed here.

Further to that, assuming this is a fix, then the patch should also be
targeted at the net tree, as opposed to the net-next tree.

	Subject: [PATCH net v2] ...

And the prefix should probably be 'be2net:'

	Subject: [PATCH net v2] be2net: ...

...

[PATCH net v2 1/1] benet: fix return value check in be_lancer_xmit_workarounds()

[Yuanjun Gong]

in be_lancer_xmit_workarounds(), it should go to label 'tx_drop'
if an unexpected value is returned by pskb_trim().

Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug")
Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 18c2fc880d09..0616b5fe241c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
 	    (lancer_chip(adapter) || BE3_chip(adapter) ||
 	     skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) {
 		ip = (struct iphdr *)ip_hdr(skb);
-		pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len));
+		if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len))))
+			goto tx_drop;
 	}
 
 	/* If vlan tag is already inlined in the packet, skip HW VLAN
-- 
2.17.1

Re: [PATCH net v2 1/1] benet: fix return value check in be_lancer_xmit_workarounds()

[Alexander H Duyck]

On Tue, 2023-07-25 at 11:27 +0800, Yuanjun Gong wrote:
> in be_lancer_xmit_workarounds(), it should go to label 'tx_drop'
> if an unexpected value is returned by pskb_trim().
> 
> Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug")
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 18c2fc880d09..0616b5fe241c 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
>  	    (lancer_chip(adapter) || BE3_chip(adapter) ||
>  	     skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) {
>  		ip = (struct iphdr *)ip_hdr(skb);
> -		pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len));
> +		if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len))))
> +			goto tx_drop;
>  	}
>  
>  	/* If vlan tag is already inlined in the packet, skip HW VLAN

I'm not sure dropping the packet is the right solution here. Based on
the description of the issue that this is a workaround for it might
make more sense to simply put out a WARN based on the failure since it
means that the tot_len field in the IP header will be modified
incorrectly and a bad IPv4 checksum will be inserted.

Re: [PATCH net v2 1/1] benet: fix return value check in be_lancer_xmit_workarounds()

[Paolo Abeni]

On Tue, 2023-07-25 at 11:00 -0700, Alexander H Duyck wrote:
> On Tue, 2023-07-25 at 11:27 +0800, Yuanjun Gong wrote:
> > in be_lancer_xmit_workarounds(), it should go to label 'tx_drop'
> > if an unexpected value is returned by pskb_trim().
> > 
> > Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug")
> > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> > ---
> >  drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> > index 18c2fc880d09..0616b5fe241c 100644
> > --- a/drivers/net/ethernet/emulex/benet/be_main.c
> > +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> > @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
> >  	    (lancer_chip(adapter) || BE3_chip(adapter) ||
> >  	     skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) {
> >  		ip = (struct iphdr *)ip_hdr(skb);
> > -		pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len));
> > +		if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len))))
> > +			goto tx_drop;
> >  	}
> >  
> >  	/* If vlan tag is already inlined in the packet, skip HW VLAN
> 
> I'm not sure dropping the packet is the right solution here. Based on
> the description of the issue that this is a workaround for it might
> make more sense to simply put out a WARN based on the failure since it
> means that the tot_len field in the IP header will be modified
> incorrectly and a bad IPv4 checksum will be inserted.

... which in turn means the packet will be dropped later, right?

Then I guess it's better to drop it now, it requires a similar amount
of code and will reduce resources usage all around.

Cheers,

Paolo

Re: [PATCH net v2 1/1] benet: fix return value check in be_lancer_xmit_workarounds()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 25 Jul 2023 11:27:26 +0800 you wrote:
> in be_lancer_xmit_workarounds(), it should go to label 'tx_drop'
> if an unexpected value is returned by pskb_trim().
> 
> Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug")
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Here is the summary with links:
  - [net,v2,1/1] benet: fix return value check in be_lancer_xmit_workarounds()
    https://git.kernel.org/netdev/net/c/5c85f7065718

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html