netdev - Re: [net-next 2/8] e1000e: Use rtnl_lock to prevent race conditions between net and pci/pm

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: <b552e8483feae4c122c862dcb8483f482f942763.camel@perches.com>
Date:   Tue, 29 Oct 2019 22:50:57 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net
Cc:     Alexander Duyck <alexander.h.duyck@...ux.intel.com>,
        netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
        Morumuri Srivalli <smorumu1@...ibm.com>,
        David Dai <zdai@...ux.vnet.ibm.com>,
        Aaron Brown <aaron.f.brown@...el.com>
Subject: Re: [net-next 2/8] e1000e: Use rtnl_lock to prevent race conditions
 between net and pci/pm

On Tue, 2019-10-29 at 21:36 -0700, Jeff Kirsher wrote:
> From: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
> 
> This patch is meant to address possible race conditions that can exist
> between network configuration and power management. A similar issue was
> fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
> netif_device_detach").
[]
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
[]
> @@ -4715,12 +4715,12 @@ int e1000e_close(struct net_device *netdev)
>  
>  	pm_runtime_get_sync(&pdev->dev);
>  
> -	if (!test_bit(__E1000_DOWN, &adapter->state)) {
> +	if (netif_device_present(netdev)) {
>  		e1000e_down(adapter, true);
>  		e1000_free_irq(adapter);
>  
>  		/* Link status message must follow this format */
> -		pr_info("%s NIC Link is Down\n", adapter->netdev->name);
> +		pr_info("%s NIC Link is Down\n", netdev->name);

This change is already a minor optimization, but perhaps
this should instead be:

	netdev_info(netdev, "NIC Link is Down\n");

And whatever pr_<level> uses should use netdev_<level>
where possible in the driver.