[PATCH 1/2] staging: fsl-dpaa2/eth: Fix skb use after free

Dan Carpenter dan.carpenter at oracle.com
Thu Jul 20 11:17:34 UTC 2017


On Thu, Jul 20, 2017 at 10:58:37AM +0000, Bogdan Purcareata wrote:
> Once a Tx frame descriptor is enqueued, an interrupt might be triggered
> to process the Tx confirmation and free the skb, hitting a memory use
> after free when updating the tx_bytes statistic based on skb->len.
> 
> Use the frame descriptor length instead.
> 
> Signed-off-by: Bogdan Purcareata <bogdan.purcareata at nxp.com>
> ---
>  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> index b9a0a31..0f3e497 100644
> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> @@ -616,7 +616,7 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
>  		free_tx_fd(priv, &fd, NULL);
>  	} else {
>  		percpu_stats->tx_packets++;
> -		percpu_stats->tx_bytes += skb->len;
> +		percpu_stats->tx_bytes += dpaa2_fd_get_len(&fd);

This feels like the wrong thing.  Can't we just save skb->len earlier
in the function and use it here?  This is the common case right?  So
we'd be saving slightly wrong information for almost every packet.

regards,
dan carpenter



More information about the devel mailing list