netdev - Re: [PATCH net] i40e: fix potential NULL pointer dereferencing

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 11 Jan 2021 20:47:38 +0100
From:   Björn Töpel <bjorn.topel@...el.com>
To:     Cristian Dumitrescu <cristian.dumitrescu@...el.com>,
        intel-wired-lan@...ts.osuosl.org
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        magnus.karlsson@...el.com, maciej.fijalkowski@...el.com
Subject: Re: [PATCH net] i40e: fix potential NULL pointer dereferencing

On 2021-01-11 19:11, Cristian Dumitrescu wrote:
> Currently, the function i40e_construct_skb_zc only frees the input xdp
> buffer when the output skb is successfully built. On error, the
> function i40e_clean_rx_irq_zc does not commit anything for the current
> packet descriptor and simply exits the packet descriptor processing
> loop, with the plan to restart the processing of this descriptor on
> the next invocation. Therefore, on error the ring next-to-clean
> pointer should not advance, the xdp i.e. *bi buffer should not be
> freed and the current buffer info should not be invalidated by setting
> *bi to NULL. Therefore, the *bi should only be set to NULL when the
> function i40e_construct_skb_zc is successful, otherwise a NULL *bi
> will be dereferenced when the work for the current descriptor is
> eventually restarted.
> 
> Fixes: 3b4f0b66c2b3 ("i40e, xsk: Migrate to new MEM_TYPE_XSK_BUFF_POOL")
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@...el.com>

Thanks for finding and fixing this, Cristian!

Acked-by: Björn Töpel <bjorn.topel@...el.com>

> ---
>   drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 47eb9c584a12..492ce213208d 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -348,12 +348,12 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
>   		 * SBP is *not* set in PRT_SBPVSI (default not set).
>   		 */
>   		skb = i40e_construct_skb_zc(rx_ring, *bi);
> -		*bi = NULL;
>   		if (!skb) {
>   			rx_ring->rx_stats.alloc_buff_failed++;
>   			break;
>   		}
>   
> +		*bi = NULL;
>   		cleaned_count++;
>   		i40e_inc_ntc(rx_ring);
>   
>