Skip to content

Commit

Permalink
qed: Fix system crash in ll2 xmit
Browse files Browse the repository at this point in the history
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 <manishc@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Manish Chopra authored and David S. Miller committed Jan 28, 2019
1 parent 327852e commit 7c81626
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions drivers/net/ethernet/qlogic/qed/qed_ll2.c
Original file line number Diff line number Diff line change
Expand Up @@ -2451,19 +2451,24 @@ static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb,
{
struct qed_ll2_tx_pkt_info pkt;
const skb_frag_t *frag;
u8 flags = 0, nr_frags;
int rc = -EINVAL, i;
dma_addr_t mapping;
u16 vlan = 0;
u8 flags = 0;

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

if (1 + skb_shinfo(skb)->nr_frags > CORE_LL2_TX_MAX_BDS_PER_PACKET) {
/* Cache number of fragments from SKB since SKB may be freed by
* the completion routine after calling qed_ll2_prepare_tx_packet()
*/
nr_frags = skb_shinfo(skb)->nr_frags;

if (1 + nr_frags > CORE_LL2_TX_MAX_BDS_PER_PACKET) {
DP_ERR(cdev, "Cannot transmit a packet with %d fragments\n",
1 + skb_shinfo(skb)->nr_frags);
1 + nr_frags);
return -EINVAL;
}

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

memset(&pkt, 0, sizeof(pkt));
pkt.num_of_bds = 1 + skb_shinfo(skb)->nr_frags;
pkt.num_of_bds = 1 + nr_frags;
pkt.vlan = vlan;
pkt.bd_flags = flags;
pkt.tx_dest = QED_LL2_TX_DEST_NW;
Expand All @@ -2496,12 +2501,17 @@ static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb,
test_bit(QED_LL2_XMIT_FLAGS_FIP_DISCOVERY, &xmit_flags))
pkt.remove_stag = true;

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

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

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

0 comments on commit 7c81626

Please sign in to comment.