netdev - Re: [PATCH v3] net/tg3: fix race condition in tg3_reset

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: <CACKFLik9h4pOPWtyaOutRwnwE+KEyO+50Cf+dMknvR2ybONTzQ@mail.gmail.com>
Date: Thu, 16 Nov 2023 13:34:07 -0800
From: Michael Chan <michael.chan@...adcom.com>
To: Thinh Tran <thinhtr@...ux.vnet.ibm.com>
Cc: mchan@...adcom.com, pavan.chebbi@...adcom.com, netdev@...r.kernel.org, 
	prashant@...adcom.com, siva.kallam@...adcom.com, drc@...ux.vnet.ibm.com, 
	venkata.sai.duggi@....com, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, davem@...emloft.net
Subject: Re: [PATCH v3] net/tg3: fix race condition in tg3_reset_task()

On Thu, Nov 16, 2023 at 7:19 AM Thinh Tran <thinhtr@...ux.vnet.ibm.com> wrote:
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 14b311196b8f..1c72ef05ab1b 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -7630,6 +7630,26 @@ static void tg3_tx_timeout(struct net_device *dev, unsigned int txqueue)
>  {
>         struct tg3 *tp = netdev_priv(dev);
>
> +       /* checking the PCI channel state for hard errors
> +        * for pci_channel_io_frozen case
> +        *   - I/O to channel is blocked.
> +        *     The EEH layer and I/O error detections will
> +        *     handle the reset procedure
> +        * for pci_channel_io_perm_failure  case
> +        *   - the PCI card is dead.
> +        *     The reset will not help
> +        * report the error for both cases and return.
> +        */
> +       if (tp->pdev->error_state == pci_channel_io_frozen) {
> +               netdev_err(dev, " %s, I/O to channel is blocked\n", __func__);
> +               return;
> +       }
> +
> +       if (tp->pdev->error_state == pci_channel_io_perm_failure) {
> +               netdev_err(dev, " %s, adapter has failed permanently!\n", __func__);
> +               return;
> +       }
> +

I think it will be better to add these 2 checks in tg3_reset_task().
We are already doing a similar check there for tp->pcierr_recovery so
it is better to centralize all the related checks in the same place.
I don't think tg3_dump_state() below will cause any problems.  We'll
probably read 0xffffffff for all registers and it will actually
confirm that the registers are not readable.

>         if (netif_msg_tx_err(tp)) {
>                 netdev_err(dev, "transmit timed out, resetting\n");
>                 tg3_dump_state(tp);
> --
> 2.25.1
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)