netdev - Re: [PATCH v1 net 3/4] net: ena: Fix incorrect descriptor free behavior

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]

Message-ID: <65a68ce7-9be3-46c0-b33a-adf5c9796bc8@amd.com>
Date: Wed, 10 Apr 2024 14:52:19 -0700
From: "Nelson, Shannon" <shannon.nelson@....com>
To: darinzon@...zon.com, David Miller <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc: "Woodhouse, David" <dwmw@...zon.com>, "Machulsky, Zorik"
 <zorik@...zon.com>, "Matushevsky, Alexander" <matua@...zon.com>,
 Saeed Bshara <saeedb@...zon.com>, "Wilson, Matt" <msw@...zon.com>,
 "Liguori, Anthony" <aliguori@...zon.com>, "Bshara, Nafea"
 <nafea@...zon.com>, "Belgazal, Netanel" <netanel@...zon.com>,
 "Saidi, Ali" <alisaidi@...zon.com>, "Herrenschmidt, Benjamin"
 <benh@...zon.com>, "Kiyanovski, Arthur" <akiyano@...zon.com>,
 "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay" <shayagr@...zon.com>,
 "Itzko, Shahar" <itzko@...zon.com>, "Abboud, Osama" <osamaabb@...zon.com>,
 "Ostrovsky, Evgeny" <evostrov@...zon.com>, "Tabachnik, Ofir"
 <ofirt@...zon.com>, Netanel Belgazal <netanel@...apurnalabs.com>,
 Sameeh Jubran <sameehj@...zon.com>
Subject: Re: [PATCH v1 net 3/4] net: ena: Fix incorrect descriptor free
 behavior

On 4/10/2024 2:13 AM, darinzon@...zon.com wrote:
> From: David Arinzon <darinzon@...zon.com>
> 
> ENA has two types of TX queues:
> - queues which only process TX packets arriving from the network stack
> - queues which only process TX packets forwarded to it by XDP_REDIRECT
>    or XDP_TX instructions
> 
> The ena_free_tx_bufs() cycles through all descriptors in a TX queue
> and unmaps + frees every descriptor that hasn't been acknowledged yet
> by the device (uncompleted TX transactions).
> The function assumes that the processed TX queue is necessarily from
> the first category listed above and ends up using napi_consume_skb()
> for descriptors belonging to an XDP specific queue.
> 
> This patch solves a bug in which, in case of a VF reset, the
> descriptors aren't freed correctly, leading to crashes.
> 
> Fixes: 548c4940b9f1 ("net: ena: Implement XDP_TX action")
> Signed-off-by: Shay Agroskin <shayagr@...zon.com>
> Signed-off-by: David Arinzon <darinzon@...zon.com>


Reviewed-by: Shannon Nelson <shannon.nelson@....com>


> ---
>   drivers/net/ethernet/amazon/ena/ena_netdev.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 59befc0f..be5acfa4 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -718,8 +718,11 @@ void ena_unmap_tx_buff(struct ena_ring *tx_ring,
>   static void ena_free_tx_bufs(struct ena_ring *tx_ring)
>   {
>          bool print_once = true;
> +       bool is_xdp_ring;
>          u32 i;
> 
> +       is_xdp_ring = ENA_IS_XDP_INDEX(tx_ring->adapter, tx_ring->qid);
> +
>          for (i = 0; i < tx_ring->ring_size; i++) {
>                  struct ena_tx_buffer *tx_info = &tx_ring->tx_buffer_info[i];
> 
> @@ -739,10 +742,15 @@ static void ena_free_tx_bufs(struct ena_ring *tx_ring)
> 
>                  ena_unmap_tx_buff(tx_ring, tx_info);
> 
> -               dev_kfree_skb_any(tx_info->skb);
> +               if (is_xdp_ring)
> +                       xdp_return_frame(tx_info->xdpf);
> +               else
> +                       dev_kfree_skb_any(tx_info->skb);
>          }
> -       netdev_tx_reset_queue(netdev_get_tx_queue(tx_ring->netdev,
> -                                                 tx_ring->qid));
> +
> +       if (!is_xdp_ring)
> +               netdev_tx_reset_queue(netdev_get_tx_queue(tx_ring->netdev,
> +                                                         tx_ring->qid));
>   }
> 
>   static void ena_free_all_tx_bufs(struct ena_adapter *adapter)
> --
> 2.40.1
> 
>