diff mbox series

[net] qede: validate non LSO skb length

Message ID 20211124092405.28834-1-manishc@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] qede: validate non LSO skb length | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: davem@davemloft.net
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 89 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Manish Chopra Nov. 24, 2021, 9:24 a.m. UTC
Although it is unlikely that stack could transmit a non LSO
skb with length > MTU, however in some cases or environment such
occurrences actually resulted into firmware asserts due to packet
length being greater than the max supported by the device (~9700B).

This patch adds the safeguard for such odd cases to avoid firmware
asserts.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Alok Prasad <palok@marvell.com>
Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qede/qede_fp.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jakub Kicinski Nov. 24, 2021, 11:38 p.m. UTC | #1
On Wed, 24 Nov 2021 01:24:05 -0800 Manish Chopra wrote:
> Although it is unlikely that stack could transmit a non LSO
> skb with length > MTU, however in some cases or environment such
> occurrences actually resulted into firmware asserts due to packet
> length being greater than the max supported by the device (~9700B).
> 
> This patch adds the safeguard for such odd cases to avoid firmware
> asserts.
> 
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: Alok Prasad <palok@marvell.com>
> Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>

Please add an appropriate Fixes tag and repost.
Manish Chopra Dec. 2, 2021, 9:19 p.m. UTC | #2
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, November 25, 2021 5:08 AM
> To: Manish Chopra <manishc@marvell.com>
> Cc: netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>; Alok Prasad
> <palok@marvell.com>; Prabhakar Kushwaha <pkushwaha@marvell.com>
> Subject: [EXT] Re: [PATCH net] qede: validate non LSO skb length
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, 24 Nov 2021 01:24:05 -0800 Manish Chopra wrote:
> > Although it is unlikely that stack could transmit a non LSO skb with
> > length > MTU, however in some cases or environment such occurrences
> > actually resulted into firmware asserts due to packet length being
> > greater than the max supported by the device (~9700B).
> >
> > This patch adds the safeguard for such odd cases to avoid firmware
> > asserts.
> >
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Signed-off-by: Alok Prasad <palok@marvell.com>
> > Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> 
> Please add an appropriate Fixes tag and repost.

Hello Jakub,

I don't really know which commit has introduced this exactly. It was probably day1 (when this driver was submitted) behavior,
just that this issue was discovered recently by some customer environment.
Let me know if you want me to put one of those initial driver commit tag here and repost ? 

Thanks,
Manish
Jakub Kicinski Dec. 3, 2021, 1:19 a.m. UTC | #3
On Thu, 2 Dec 2021 21:19:26 +0000 Manish Chopra wrote:
> > On Wed, 24 Nov 2021 01:24:05 -0800 Manish Chopra wrote:  
> > > Although it is unlikely that stack could transmit a non LSO skb with
> > > length > MTU, however in some cases or environment such occurrences
> > > actually resulted into firmware asserts due to packet length being
> > > greater than the max supported by the device (~9700B).
> > >
> > > This patch adds the safeguard for such odd cases to avoid firmware
> > > asserts.
> > >
> > > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > > Signed-off-by: Alok Prasad <palok@marvell.com>
> > > Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> > > Signed-off-by: Ariel Elior <aelior@marvell.com>  
> > 
> > Please add an appropriate Fixes tag and repost.  
> 
> Hello Jakub,
> 
> I don't really know which commit has introduced this exactly. It was
> probably day1 (when this driver was submitted) behavior, just that
> this issue was discovered recently by some customer environment. Let
> me know if you want me to put one of those initial driver commit tag
> here and repost ? 

Yes, that'd be best. This way any sort of automation for tracking which
fixes need to be backported will have clear signal that this fix
applies to all the kernels.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index e113fbd56e86..5ea9cb4311a1 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -1644,6 +1644,13 @@  netdev_tx_t qede_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 			data_split = true;
 		}
 	} else {
+		if (unlikely(skb->len > ETH_TX_MAX_NON_LSO_PKT_LEN)) {
+			DP_ERR(edev, "Unexpected non LSO skb length = 0x%x\n", skb->len);
+			qede_free_failed_tx_pkt(txq, first_bd, 0, false);
+			qede_update_tx_producer(txq);
+			return NETDEV_TX_OK;
+		}
+
 		val |= ((skb->len & ETH_TX_DATA_1ST_BD_PKT_LEN_MASK) <<
 			 ETH_TX_DATA_1ST_BD_PKT_LEN_SHIFT);
 	}