Skip to content

Commit 7c81626

Browse files
manishc88davem330
authored andcommitted
qed: Fix system crash in ll2 xmit
Cache number of fragments in the skb locally as in case of linear skb (with zero fragments), tx completion (or freeing of skb) may happen before driver tries to get number of frgaments from the skb which could lead to stale access to an already freed skb. Signed-off-by: Manish Chopra <[email protected]> Signed-off-by: Ariel Elior <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 327852e commit 7c81626

File tree

1 file changed

+15
-5
lines changed

1 file changed

+15
-5
lines changed

drivers/net/ethernet/qlogic/qed/qed_ll2.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2451,19 +2451,24 @@ static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb,
24512451
{
24522452
struct qed_ll2_tx_pkt_info pkt;
24532453
const skb_frag_t *frag;
2454+
u8 flags = 0, nr_frags;
24542455
int rc = -EINVAL, i;
24552456
dma_addr_t mapping;
24562457
u16 vlan = 0;
2457-
u8 flags = 0;
24582458

24592459
if (unlikely(skb->ip_summed != CHECKSUM_NONE)) {
24602460
DP_INFO(cdev, "Cannot transmit a checksummed packet\n");
24612461
return -EINVAL;
24622462
}
24632463

2464-
if (1 + skb_shinfo(skb)->nr_frags > CORE_LL2_TX_MAX_BDS_PER_PACKET) {
2464+
/* Cache number of fragments from SKB since SKB may be freed by
2465+
* the completion routine after calling qed_ll2_prepare_tx_packet()
2466+
*/
2467+
nr_frags = skb_shinfo(skb)->nr_frags;
2468+
2469+
if (1 + nr_frags > CORE_LL2_TX_MAX_BDS_PER_PACKET) {
24652470
DP_ERR(cdev, "Cannot transmit a packet with %d fragments\n",
2466-
1 + skb_shinfo(skb)->nr_frags);
2471+
1 + nr_frags);
24672472
return -EINVAL;
24682473
}
24692474

@@ -2485,7 +2490,7 @@ static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb,
24852490
}
24862491

24872492
memset(&pkt, 0, sizeof(pkt));
2488-
pkt.num_of_bds = 1 + skb_shinfo(skb)->nr_frags;
2493+
pkt.num_of_bds = 1 + nr_frags;
24892494
pkt.vlan = vlan;
24902495
pkt.bd_flags = flags;
24912496
pkt.tx_dest = QED_LL2_TX_DEST_NW;
@@ -2496,12 +2501,17 @@ static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb,
24962501
test_bit(QED_LL2_XMIT_FLAGS_FIP_DISCOVERY, &xmit_flags))
24972502
pkt.remove_stag = true;
24982503

2504+
/* qed_ll2_prepare_tx_packet() may actually send the packet if
2505+
* there are no fragments in the skb and subsequently the completion
2506+
* routine may run and free the SKB, so no dereferencing the SKB
2507+
* beyond this point unless skb has any fragments.
2508+
*/
24992509
rc = qed_ll2_prepare_tx_packet(&cdev->hwfns[0], cdev->ll2->handle,
25002510
&pkt, 1);
25012511
if (rc)
25022512
goto err;
25032513

2504-
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
2514+
for (i = 0; i < nr_frags; i++) {
25052515
frag = &skb_shinfo(skb)->frags[i];
25062516

25072517
mapping = skb_frag_dma_map(&cdev->pdev->dev, frag, 0,

0 commit comments

Comments
 (0)