* [PATCH] amd-xgbe: set skb to NULL after freeing it
@ 2018-11-28 8:09 Pan Bian
2018-11-28 18:49 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Pan Bian @ 2018-11-28 8:09 UTC (permalink / raw)
To: Tom Lendacky, David S. Miller; +Cc: netdev, linux-kernel, Pan Bian
The buffer skb is freed via dev_kfree_skb in a loop. skb may be used
again in the next iteration, resulting in a use-after-free bug. To fix
this, the patch set skb to NULL after dev_kfree_skb(skb).
Signed-off-by: Pan Bian <bianpan2016@163.com>
---
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 0cc911f..ac6b82d 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -2754,6 +2754,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
netif_err(pdata, rx_err, netdev,
"error in received packet\n");
dev_kfree_skb(skb);
+ skb = NULL;
goto next_packet;
}
@@ -2806,6 +2807,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
netif_err(pdata, rx_err, netdev,
"packet length exceeds configured MTU\n");
dev_kfree_skb(skb);
+ skb = NULL;
goto next_packet;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] amd-xgbe: set skb to NULL after freeing it
2018-11-28 8:09 [PATCH] amd-xgbe: set skb to NULL after freeing it Pan Bian
@ 2018-11-28 18:49 ` David Miller
2018-11-29 7:37 ` PanBian
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2018-11-28 18:49 UTC (permalink / raw)
To: bianpan2016; +Cc: thomas.lendacky, netdev, linux-kernel
From: Pan Bian <bianpan2016@163.com>
Date: Wed, 28 Nov 2018 16:09:45 +0800
> @@ -2754,6 +2754,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
> netif_err(pdata, rx_err, netdev,
> "error in received packet\n");
> dev_kfree_skb(skb);
> + skb = NULL;
> goto next_packet;
> }
>
> @@ -2806,6 +2807,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
> netif_err(pdata, rx_err, netdev,
> "packet length exceeds configured MTU\n");
> dev_kfree_skb(skb);
> + skb = NULL;
> goto next_packet;
> }
>
> --
> 2.7.4
If either of these two cases are executed, it means that it must be
the case that:
!last || context_next
is FALSE, therefore the conditional in the tail code of this function:
/* Check if we need to save state before leaving */
if (received && (!last || context_next)) {
rdata = XGBE_GET_DESC_DATA(ring, ring->cur);
rdata->state_saved = 1;
rdata->state.skb = skb;
rdata->state.len = len;
rdata->state.error = error;
}
will not trigger, and therefore we will not record the free'd SKB.
And if we iterate the loop again, it is guaranteed that 'skb' will
be set to a new packet or NULL.
So there is no problem here.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] amd-xgbe: set skb to NULL after freeing it
2018-11-28 18:49 ` David Miller
@ 2018-11-29 7:37 ` PanBian
0 siblings, 0 replies; 3+ messages in thread
From: PanBian @ 2018-11-29 7:37 UTC (permalink / raw)
To: David Miller; +Cc: thomas.lendacky, netdev, linux-kernel
On Wed, Nov 28, 2018 at 10:49:31AM -0800, David Miller wrote:
> From: Pan Bian <bianpan2016@163.com>
> Date: Wed, 28 Nov 2018 16:09:45 +0800
>
> If either of these two cases are executed, it means that it must be
> the case that:
>
> !last || context_next
>
> is FALSE, therefore the conditional in the tail code of this function:
>
> /* Check if we need to save state before leaving */
> if (received && (!last || context_next)) {
> rdata = XGBE_GET_DESC_DATA(ring, ring->cur);
> rdata->state_saved = 1;
> rdata->state.skb = skb;
> rdata->state.len = len;
> rdata->state.error = error;
> }
>
> will not trigger, and therefore we will not record the free'd SKB.
>
> And if we iterate the loop again, it is guaranteed that 'skb' will
> be set to a new packet or NULL.
>
> So there is no problem here.
Got it. Sorry for my misunderstanding. Thank you and Tom for the
detailed explanations.
Pan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-29 7:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 8:09 [PATCH] amd-xgbe: set skb to NULL after freeing it Pan Bian
2018-11-28 18:49 ` David Miller
2018-11-29 7:37 ` PanBian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).