From: Saeed Mahameed <sae...@mellanox.com>
Date: Thu, 17 Dec 2015 14:35:32 +0200

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> index 1341b1d..0fcfe64 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> @@ -165,6 +165,7 @@ static netdev_tx_t mlx5e_sq_xmit(struct mlx5e_sq *sq, 
> struct sk_buff *skb)
>       struct mlx5_wqe_eth_seg  *eseg = &wqe->eth;
>       struct mlx5_wqe_data_seg *dseg;
>  
> +     unsigned char *skb_data_orig = skb->data;
>       u8  opcode = MLX5_OPCODE_SEND;
>       dma_addr_t dma_addr = 0;
>       bool bf = false;
> @@ -263,6 +264,7 @@ static netdev_tx_t mlx5e_sq_xmit(struct mlx5e_sq *sq, 
> struct sk_buff *skb)
>       cseg->opmod_idx_opcode = cpu_to_be32((sq->pc << 8) | opcode);
>       cseg->qpn_ds           = cpu_to_be32((sq->sqn << 8) | ds_cnt);
>  
> +     skb_push(skb, skb->data - skb_data_orig);
>       sq->skb[pi] = skb;
>  
>       MLX5E_TX_SKB_CB(skb)->num_wqebbs = DIV_ROUND_UP(ds_cnt,

And in the middle of this we have:

                skb_pull_inline(skb, ihs);

This is looks illegal.

You must not modify the data pointers of any SKB that you receive for
sending via ->ndo_start_xmit() unless you know that absolutely you are
the one and only reference that exists to that SKB.

And exactly for the case you are trying to "fix" here, you do not.  If
the SKB is cloned, or has an elevated users count, someone else can be
looking at it exactly at the same time you are messing with the data
pointers.

I bet mlx4 has this bug too.

You must fix this properly, by keeping track of an offset or similar
internally to your driver, rather than changing the SKB data pointers.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to