netdev - Re: [PATCH net-next] net: ena: resolve WARN

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: <20250312143929.GT4159220@kernel.org>
Date: Wed, 12 Mar 2025 15:39:29 +0100
From: Simon Horman <horms@...nel.org>
To: David Arinzon <darinzon@...zon.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org, Ahmed Zaki <ahmed.zaki@...el.com>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Richard Cochran <richardcochran@...il.com>,
	"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>,
	"Schmeilin, Evgeny" <evgenys@...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>,
	"Bernstein, Amit" <amitbern@...zon.com>,
	"Agroskin, Shay" <shayagr@...zon.com>,
	"Ostrovsky, Evgeny" <evostrov@...zon.com>,
	"Tabachnik, Ofir" <ofirt@...zon.com>
Subject: Re: [PATCH net-next] net: ena: resolve WARN_ON when freeing IRQs

On Mon, Mar 10, 2025 at 10:01:48AM +0200, David Arinzon wrote:
> When IRQs are freed, a WARN_ON is triggered as the
> affinity notifier is not released.
> This results in the below stack trace:
> 
> [  484.544586]  ? __warn+0x84/0x130
> [  484.544843]  ? free_irq+0x5c/0x70
> [  484.545105]  ? report_bug+0x18a/0x1a0
> [  484.545390]  ? handle_bug+0x53/0x90
> [  484.545664]  ? exc_invalid_op+0x14/0x70
> [  484.545959]  ? asm_exc_invalid_op+0x16/0x20
> [  484.546279]  ? free_irq+0x5c/0x70
> [  484.546545]  ? free_irq+0x10/0x70
> [  484.546807]  ena_free_io_irq+0x5f/0x70 [ena]
> [  484.547138]  ena_down+0x250/0x3e0 [ena]
> [  484.547435]  ena_destroy_device+0x118/0x150 [ena]
> [  484.547796]  __ena_shutoff+0x5a/0xe0 [ena]
> [  484.548110]  pci_device_remove+0x3b/0xb0
> [  484.548412]  device_release_driver_internal+0x193/0x200
> [  484.548804]  driver_detach+0x44/0x90
> [  484.549084]  bus_remove_driver+0x69/0xf0
> [  484.549386]  pci_unregister_driver+0x2a/0xb0
> [  484.549717]  ena_cleanup+0xc/0x130 [ena]
> [  484.550021]  __do_sys_delete_module.constprop.0+0x176/0x310
> [  484.550438]  ? syscall_trace_enter+0xfb/0x1c0
> [  484.550782]  do_syscall_64+0x5b/0x170
> [  484.551067]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> 
> Adding a call to `netif_napi_set_irq` with -1 as the IRQ index,
> which frees the notifier.
> 
> Fixes: de340d8206bf ("net: ena: use napi's aRFS rmap notifers")
> Signed-off-by: David Arinzon <darinzon@...zon.com>

Thanks David,

I agree that having a notifier set should result in a WARN_ON,
and that your patch addresses this problem.

So, the nit below not withstanding this looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>

> ---
>  drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 6aab85a7..9e007c60 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -1716,8 +1716,12 @@ static void ena_free_io_irq(struct ena_adapter *adapter)
>  	int i;
>  
>  	for (i = ENA_IO_IRQ_FIRST_IDX; i < ENA_MAX_MSIX_VEC(io_queue_count); i++) {
> +		struct ena_napi *ena_napi;
> +
>  		irq = &adapter->irq_tbl[i];
>  		irq_set_affinity_hint(irq->vector, NULL);
> +		ena_napi = (struct ena_napi *)irq->data;

nit: I don't think it is necessary to explicitly cast irq->data
     to the pointer type of ena_napi because irq->data is a void *.

> +		netif_napi_set_irq(&ena_napi->napi, -1);
>  		free_irq(irq->vector, irq->data);
>  	}
>  }
> -- 
> 2.47.1
>