| From 1630770885204931f3711737b4d6da1728071c68 Mon Sep 17 00:00:00 2001 |
| From: Manish Chopra <manishc@marvell.com> |
| Date: Mon, 28 Jan 2019 10:05:07 -0800 |
| Subject: qed: Fix system crash in ll2 xmit |
| |
| [ Upstream commit 7c81626a3c37e4ac320b8ad785694ba498f24794 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/ethernet/qlogic/qed/qed_ll2.c | 20 +++++++++++++++----- |
| 1 file changed, 15 insertions(+), 5 deletions(-) |
| |
| diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c |
| index 92cd8abeb41d..015de1e0addd 100644 |
| --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c |
| +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c |
| @@ -2430,19 +2430,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; |
| } |
| |
| @@ -2464,7 +2469,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; |
| @@ -2475,12 +2480,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, |
| -- |
| 2.19.1 |
| |