Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <99f1f2d1-8fb0-4e56-9820-86254ce8bd02@intel.com>
Date:   Thu, 26 Oct 2023 12:47:52 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net] hv_netvsc: fix race of netvsc and VF
 register_netdevice
Content-Language: en-US
To:     Haiyang Zhang <haiyangz@microsoft.com>,
        <linux-hyperv@vger.kernel.org>, <netdev@vger.kernel.org>
CC:     <kys@microsoft.com>, <wei.liu@kernel.org>, <decui@microsoft.com>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <davem@davemloft.net>, <linux-kernel@vger.kernel.org>,
        <stable@vger.kernel.org>
References: <1698268592-20373-1-git-send-email-haiyangz@microsoft.com>
From:   Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <1698268592-20373-1-git-send-email-haiyangz@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bG4yMWd6c2EyNVpIMUJ1TXR0U3VUeHB5Qldjeks1SXFFZFg0bmJEYm9xbnEy?=
 =?utf-8?B?SFdpUEp1QTlnQ0JsdmVsQ1k1T1AxdVo0dFFNVXpMaUdWMHNtV3J0eU94ZHlJ?=
 =?utf-8?B?dklzWGNHUHgxRzFnVEo5S2UxR3UwMnc4dGJyd1Qyc3hlV252U0hxa1NQL0dS?=
 =?utf-8?B?eXVKdUNTbldGOHpET09JbFFCeTlBN1Eralh0M1dwRXFRakpRTjE0VU84dGdn?=
 =?utf-8?B?SThld04rbEhFUTdKbENQT0RuVzIwRVZrZW0yS3JDUnlvMUVaZ3F3RENvSXVq?=
 =?utf-8?B?eGRVYXc5YXllZnNYVmF2cjhpdFpLZDRkVWtZUnZBdFYwUisxZFNnNUtvWVox?=
 =?utf-8?B?SFIyMGVDUGtlYmFTWmYyd0g4ckp3ZFlOS0psLzVHcmxkblh0VGZjbWV4Si9K?=
 =?utf-8?B?MVN3NU9kbVdHRFphSGdMUlM5dW9jUjFPM3dtYW00OWM4ZGlGOWNLQjdNc2F2?=
 =?utf-8?B?TDRtMUlOdTVPZEFRTzc5OXJBSThjRW9LMlhMMGJSUlVjU2tBVWIwUHd3a0lC?=
 =?utf-8?B?aWVON3Q5TVpsRHltTTNvK1pkK05MbXNteXpBM0VCSWtUOVZmbGNWMWlUcFRF?=
 =?utf-8?B?cmpvTnBJMkpHZXQ4eTQ5bzVrdkNPNHZZU0lmQlduQi9wb3pLMmw0aUl1a0JK?=
 =?utf-8?B?aDYreDJUR1FaRkxSZEszSFkyUzBOR3hVd3o5N1JCMVphb2xzYXBrV3VqNEd5?=
 =?utf-8?B?V2hZUngxenBUOVQwNmxGWVlkS0J1UWdtdmxRaWJKWXRKUmp6WjFSUTZUTlVj?=
 =?utf-8?B?Y0xtV2NBcWxhSU93T3VDeElEQlBjOXdDTmhzdllmd3FJY2xza2JlbHp1QkdM?=
 =?utf-8?B?WnNzNUcyMnpLT2hXNjBXeXJhd0huTGxzQlZhMXM2My9NbXJUQ080NE1LQ0lp?=
 =?utf-8?B?dU1lLzNlZXkyeitpNjNlam1MNW9wTmw3K2V4bUNiQ3dxYlRwcFR2Nmt1Wnls?=
 =?utf-8?B?cmFSc0VXUXBaWThXcnptY1ovRXZDQXUyVG1VbkdvUGhtYldMbEJlTjJCY2JZ?=
 =?utf-8?B?dklyS0FuNmIzQ01nQW5mMjYyODgzVWo2ekpXalltYmVuUWZ6SkpDcndUQklX?=
 =?utf-8?B?QzczY2M4N3lzSStDckR6aVN0WHlRYU41dnJxMnE0Lzl0ZnpDOFQvRE1ic3NM?=
 =?utf-8?B?dzVKZkNybnBjMnNJZEEzV0w4aHI1TGVITEhFNUk0dUpmNS9MOGpDNEdQRDFm?=
 =?utf-8?B?SGo5LzcvZXBOSDZDRmNEQVBQbU84SSs4d0pYdHVKZ0lCU0l3Nk5OVDd3OSty?=
 =?utf-8?B?c1RWWVc2bVI2cWQ3WnNrbkoxYnc2K2JnbWV4bFVySllYMUhhdy9qRHg5Vm9O?=
 =?utf-8?B?UWc3Ylh0QnlWWlk2SWFpcE5qQWNoTmhhK1VSbXY4VmcvbFJIbmtUdXhPRm9O?=
 =?utf-8?B?NGxYSmliVU5ET3ZIS0VkU0VBU3d0Q1VRcHdhSzdBN29BYmJqY20vbi9KbUI2?=
 =?utf-8?B?YkZRRjlrZ0YxR2FyRDJHZE41RWNQN1p3d2JWMVVWZlFDOE5yRUZJQjFsZk1M?=
 =?utf-8?B?d2IzdmFkL3VZRU1nQnBPczlGOGg5aFlTS2tnejFzY053cERBZERVYU5MTkU4?=
 =?utf-8?B?aHVyZlY2aWJvTTNXbHRvSGppcTNaODFsWklONTF1WXZ0Z1Y0MndTejl6UmJx?=
 =?utf-8?B?eUxjR2JLS2JtV3VoVXl0WWE2aVBTU2xmSGd0ZjZuUCtuVGQrdWc0cCtwYU1u?=
 =?utf-8?B?aDRTM3VvNXpmMTRJSmh5SnhDZUx0NmxXMGdUTXBibVBIRnVVd2NxMWMvZDlj?=
 =?utf-8?B?UVNyckpnSFJ4c3lJSFo1SUNJZ1RobzZPUFQraTEzQzgwdm1pck1VVVBlc25n?=
 =?utf-8?B?Q3ZSTHVMNDJhVW5OTkV6aHhqODNjeVZnQ2ZKZ1cvMXRockhheGQyaFgzbnZ4?=
 =?utf-8?B?WGRXZ3dPK29oY0JzWW1SdzBHc2NjZ3NrdlFJcnRjejlGdDVvVTRFbmxkTGdZ?=
 =?utf-8?B?bW1QczB2bFNnMm1PbEtsSXVQMHp2eXI3SFhva2dhcHVhbVpaVVJ3ekhZc29y?=
 =?utf-8?B?NUEwNXM4MjZxSkJaU3VwdzNhU1lSamYySzZBbnhTUXRsYTgwbUhsMzFCQ3Na?=
 =?utf-8?B?SHhvT2p3VEZCN1h3NldYZUtUcUV3dnJXOXA3M05UYnBuY0tWays2c0FZZlQ2?=
 =?utf-8?B?ZnA0WjlRVk9mMW92L0txYUJOUzVQcFJoZk5VYmIvSk9pcHBuK3FXc1EwWXRX?=
 =?utf-8?B?aUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0cc746e7-af5d-4561-e6b3-08dbd6110520
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 10:47:59.4559
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gWDQs0hshiUpJhvsX9uLWNsvN2U5OLX2hVh0Ly78OWgoLuZHM0rRkm4BIy5h2S+k+FvbxgMChlCsb5HI2nkUCgGqs89eNVPwidkN/o/jWnM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB7303
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 26 Oct 2023 03:48:15 -0700 (PDT)


On 25.10.2023 23:16, Haiyang Zhang wrote:
> The rtnl lock also needs to be held before rndis_filter_device_add()
> which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> VF NIC offering and registering. If VF NIC finished register_netdev()
> earlier it may cause name based config failure.
> 
> To fix this issue, move the call to rtnl_lock() before
> rndis_filter_device_add(), so VF will be registered later than netvsc
> / synthetic NIC, and gets a name numbered (ethX) after netvsc.
> 
> And, move register_netdevice_notifier() earlier, so the call back
> function is set before probing.
> 
> Cc: stable@vger.kernel.org
> Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier in netvsc_probe()")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> 
> ---
>  drivers/net/hyperv/netvsc_drv.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 3ba3c8fb28a5..feca1391f756 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2531,15 +2531,6 @@ static int netvsc_probe(struct hv_device *dev,
>  		goto devinfo_failed;
>  	}
>  
> -	nvdev = rndis_filter_device_add(dev, device_info);
> -	if (IS_ERR(nvdev)) {
> -		ret = PTR_ERR(nvdev);
> -		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> -		goto rndis_failed;
> -	}
> -
> -	eth_hw_addr_set(net, device_info->mac_adr);
> -
>  	/* We must get rtnl lock before scheduling nvdev->subchan_work,
>  	 * otherwise netvsc_subchan_work() can get rtnl lock first and wait
>  	 * all subchannels to show up, but that may not happen because
> @@ -2547,9 +2538,23 @@ static int netvsc_probe(struct hv_device *dev,
>  	 * -> ... -> device_add() -> ... -> __device_attach() can't get
>  	 * the device lock, so all the subchannels can't be processed --
>  	 * finally netvsc_subchan_work() hangs forever.
> +	 *
> +	 * The rtnl lock also needs to be held before rndis_filter_device_add()
> +	 * which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> +	 * VF NIC offering and registering. If VF NIC finished register_netdev()
> +	 * earlier it may cause name based config failure.
>  	 */
>  	rtnl_lock();
>  
> +	nvdev = rndis_filter_device_add(dev, device_info);
> +	if (IS_ERR(nvdev)) {
> +		ret = PTR_ERR(nvdev);
> +		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> +		goto rndis_failed;

In case of error rtnl won't be unlocked.

> +	}
> +
> +	eth_hw_addr_set(net, device_info->mac_adr);
> +
>  	if (nvdev->num_chn > 1)
>  		schedule_work(&nvdev->subchan_work);
>  
> @@ -2788,11 +2793,14 @@ static int __init netvsc_drv_init(void)
>  	}
>  	netvsc_ring_bytes = ring_size * PAGE_SIZE;
>  
> +	register_netdevice_notifier(&netvsc_netdev_notifier);
> +
>  	ret = vmbus_driver_register(&netvsc_drv);
> -	if (ret)
> +	if (ret) {
> +		unregister_netdevice_notifier(&netvsc_netdev_notifier);
>  		return ret;
> +	}
>  
> -	register_netdevice_notifier(&netvsc_netdev_notifier);
>  	return 0;
>  }
>