Skip to content

Commit

Permalink
bnxt: prevent skb UAF after handing over to PTP worker
Browse files Browse the repository at this point in the history
When reading the timestamp is required bnxt_tx_int() hands
over the ownership of the completed skb to the PTP worker.
The skb should not be used afterwards, as the worker may
run before the rest of our code and free the skb, leading
to a use-after-free.

Since dev_kfree_skb_any() accepts NULL make the loss of
ownership more obvious and set skb to NULL.

Fixes: 83bb623 ("bnxt_en: Transmit and retrieve packet timestamps")
Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://lore.kernel.org/r/20220921201005.335390-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Sep 22, 2022
1 parent 3aac7ad commit c31f26c
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions drivers/net/ethernet/broadcom/bnxt/bnxt.c
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,6 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)

for (i = 0; i < nr_pkts; i++) {
struct bnxt_sw_tx_bd *tx_buf;
bool compl_deferred = false;
struct sk_buff *skb;
int j, last;

Expand All @@ -668,6 +667,8 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
skb = tx_buf->skb;
tx_buf->skb = NULL;

tx_bytes += skb->len;

if (tx_buf->is_push) {
tx_buf->is_push = 0;
goto next_tx_int;
Expand All @@ -688,8 +689,9 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
}
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
if (bp->flags & BNXT_FLAG_CHIP_P5) {
/* PTP worker takes ownership of the skb */
if (!bnxt_get_tx_ts_p5(bp, skb))
compl_deferred = true;
skb = NULL;
else
atomic_inc(&bp->ptp_cfg->tx_avail);
}
Expand All @@ -698,9 +700,7 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
next_tx_int:
cons = NEXT_TX(cons);

tx_bytes += skb->len;
if (!compl_deferred)
dev_kfree_skb_any(skb);
dev_kfree_skb_any(skb);
}

netdev_tx_completed_queue(txq, nr_pkts, tx_bytes);
Expand Down

0 comments on commit c31f26c

Please sign in to comment.