netdev - Re: [PATCH] net: nvidia: forcedeth: Fix two possible concurrency use-after-free bugs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Jan 2019 20:54:20 +0800
From:   Zhu Yanjun <yanjun.zhu@...cle.com>
To:     Jia-Ju Bai <baijiaju1990@...il.com>, davem@...emloft.net,
        keescook@...omium.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: nvidia: forcedeth: Fix two possible concurrency
 use-after-free bugs


在 2019/1/8 20:45, Jia-Ju Bai 写道:
> In drivers/net/ethernet/nvidia/forcedeth.c, the functions
> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently
> executed with nv_poll_controller().
>
> nv_start_xmit
>    line 2321: prev_tx_ctx->skb = skb;
>
> nv_start_xmit_optimized
>    line 2479: prev_tx_ctx->skb = skb;
>
> nv_poll_controller
>    nv_do_nic_poll
>      line 4134: spin_lock(&np->lock);
>      nv_drain_rxtx
>        nv_drain_tx
>          nv_release_txskb
>            line 2004: dev_kfree_skb_any(tx_skb->skb);
>
> Thus, two possible concurrency use-after-free bugs may occur.
>
> To fix these possible bugs,


Does this really occur? Can you reproduce this ?


>   the calls to spin_lock_irqsave() in
> nv_start_xmit() and nv_start_xmit_optimized() are moved to the
> front of "prev_tx_ctx->skb = skb;"
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@...il.com>
> ---
>   drivers/net/ethernet/nvidia/forcedeth.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
> index 1d9b0d44ddb6..48fa5a0bd2cb 100644
> --- a/drivers/net/ethernet/nvidia/forcedeth.c
> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
> @@ -2317,6 +2317,8 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
>   	/* set last fragment flag  */
>   	prev_tx->flaglen |= cpu_to_le32(tx_flags_extra);
>   
> +	spin_lock_irqsave(&np->lock, flags);
> +
>   	/* save skb in this slot's context area */
>   	prev_tx_ctx->skb = skb;
>   
> @@ -2326,8 +2328,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
>   		tx_flags_extra = skb->ip_summed == CHECKSUM_PARTIAL ?
>   			 NV_TX2_CHECKSUM_L3 | NV_TX2_CHECKSUM_L4 : 0;
>   
> -	spin_lock_irqsave(&np->lock, flags);
> -
>   	/* set tx flags */
>   	start_tx->flaglen |= cpu_to_le32(tx_flags | tx_flags_extra);
>   
> @@ -2475,6 +2475,8 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
>   	/* set last fragment flag  */
>   	prev_tx->flaglen |= cpu_to_le32(NV_TX2_LASTPACKET);
>   
> +	spin_lock_irqsave(&np->lock, flags);
> +
>   	/* save skb in this slot's context area */
>   	prev_tx_ctx->skb = skb;
>   
> @@ -2491,8 +2493,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
>   	else
>   		start_tx->txvlan = 0;
>   
> -	spin_lock_irqsave(&np->lock, flags);
> -
>   	if (np->tx_limit) {
>   		/* Limit the number of outstanding tx. Setup all fragments, but
>   		 * do not set the VALID bit on the first descriptor. Save a pointer