From: "Michael Kelley (LINUX)" <mikelley@microsoft.com> To: Tianyu Lan <ltykernel@gmail.com>, KY Srinivasan <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, "wei.liu@kernel.org" <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>, "tglx@linutronix.de" <tglx@linutronix.de>, "mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de" <bp@alien8.de>, "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>, "x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>, "davem@davemloft.net" <davem@davemloft.net>, "kuba@kernel.org" <kuba@kernel.org>, "hch@infradead.org" <hch@infradead.org>, "m.szyprowski@samsung.com" <m.szyprowski@samsung.com>, "robin.murphy@arm.com" <robin.murphy@arm.com> Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "netdev@vger.kernel.org" <netdev@vger.kernel.org> Subject: RE: [PATCH] Netvsc: Call hv_unmap_memory() in the netvsc_device_remove() Date: Wed, 2 Feb 2022 17:05:20 +0000 [thread overview] Message-ID: <MWHPR21MB15935F58E55D05A171AE9ED4D7279@MWHPR21MB1593.namprd21.prod.outlook.com> (raw) In-Reply-To: <20220201163211.467423-1-ltykernel@gmail.com> From: Tianyu Lan <ltykernel@gmail.com> Sent: Tuesday, February 1, 2022 8:32 AM > > netvsc_device_remove() calls vunmap() inside which should not be > called in the interrupt context. Current code calls hv_unmap_memory() > in the free_netvsc_device() which is rcu callback and maybe called > in the interrupt context. This will trigger BUG_ON(in_interrupt()) > in the vunmap(). Fix it via moving hv_unmap_memory() to netvsc_device_ > remove(). I think this change can fail to call hv_unmap_memory() in an error case. If netvsc_init_buf() fails after hv_map_memory() succeeds for the receive buffer or for the send buffer, no corresponding hv_unmap_memory() will be done. The failure in netvsc_init_buf() will cause netvsc_connect_vsp() to fail, so netvsc_add_device() will "goto close" where free_netvsc_device() will be called. But free_netvsc_device() no longer calls hv_unmap_memory(), so it won't ever happen. netvsc_device_remove() is never called in this case because netvsc_add_device() failed. Michael > > Fixes: 846da38de0e8 ("net: netvsc: Add Isolation VM support for netvsc driver") > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > --- > drivers/net/hyperv/netvsc.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index afa81a9480cc..f989f920d4ce 100644 > --- a/drivers/net/hyperv/netvsc.c > +++ b/drivers/net/hyperv/netvsc.c > @@ -154,19 +154,15 @@ static void free_netvsc_device(struct rcu_head *head) > > kfree(nvdev->extension); > > - if (nvdev->recv_original_buf) { > - hv_unmap_memory(nvdev->recv_buf); > + if (nvdev->recv_original_buf) > vfree(nvdev->recv_original_buf); > - } else { > + else > vfree(nvdev->recv_buf); > - } > > - if (nvdev->send_original_buf) { > - hv_unmap_memory(nvdev->send_buf); > + if (nvdev->send_original_buf) > vfree(nvdev->send_original_buf); > - } else { > + else > vfree(nvdev->send_buf); > - } > > bitmap_free(nvdev->send_section_map); > > @@ -765,6 +761,12 @@ void netvsc_device_remove(struct hv_device *device) > netvsc_teardown_send_gpadl(device, net_device, ndev); > } > > + if (net_device->recv_original_buf) > + hv_unmap_memory(net_device->recv_buf); > + > + if (net_device->send_original_buf) > + hv_unmap_memory(net_device->send_buf); > + > /* Release all resources */ > free_netvsc_device_rcu(net_device); > } > -- > 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: "Michael Kelley \(LINUX\) via iommu" <iommu@lists.linux-foundation.org> To: Tianyu Lan <ltykernel@gmail.com>, KY Srinivasan <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, "wei.liu@kernel.org" <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>, "tglx@linutronix.de" <tglx@linutronix.de>, "mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de" <bp@alien8.de>, "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>, "x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>, "davem@davemloft.net" <davem@davemloft.net>, "kuba@kernel.org" <kuba@kernel.org>, "hch@infradead.org" <hch@infradead.org>, "m.szyprowski@samsung.com" <m.szyprowski@samsung.com>, "robin.murphy@arm.com" <robin.murphy@arm.com> Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, Tianyu Lan <Tianyu.Lan@microsoft.com>, "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: RE: [PATCH] Netvsc: Call hv_unmap_memory() in the netvsc_device_remove() Date: Wed, 2 Feb 2022 17:05:20 +0000 [thread overview] Message-ID: <MWHPR21MB15935F58E55D05A171AE9ED4D7279@MWHPR21MB1593.namprd21.prod.outlook.com> (raw) In-Reply-To: <20220201163211.467423-1-ltykernel@gmail.com> From: Tianyu Lan <ltykernel@gmail.com> Sent: Tuesday, February 1, 2022 8:32 AM > > netvsc_device_remove() calls vunmap() inside which should not be > called in the interrupt context. Current code calls hv_unmap_memory() > in the free_netvsc_device() which is rcu callback and maybe called > in the interrupt context. This will trigger BUG_ON(in_interrupt()) > in the vunmap(). Fix it via moving hv_unmap_memory() to netvsc_device_ > remove(). I think this change can fail to call hv_unmap_memory() in an error case. If netvsc_init_buf() fails after hv_map_memory() succeeds for the receive buffer or for the send buffer, no corresponding hv_unmap_memory() will be done. The failure in netvsc_init_buf() will cause netvsc_connect_vsp() to fail, so netvsc_add_device() will "goto close" where free_netvsc_device() will be called. But free_netvsc_device() no longer calls hv_unmap_memory(), so it won't ever happen. netvsc_device_remove() is never called in this case because netvsc_add_device() failed. Michael > > Fixes: 846da38de0e8 ("net: netvsc: Add Isolation VM support for netvsc driver") > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > --- > drivers/net/hyperv/netvsc.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index afa81a9480cc..f989f920d4ce 100644 > --- a/drivers/net/hyperv/netvsc.c > +++ b/drivers/net/hyperv/netvsc.c > @@ -154,19 +154,15 @@ static void free_netvsc_device(struct rcu_head *head) > > kfree(nvdev->extension); > > - if (nvdev->recv_original_buf) { > - hv_unmap_memory(nvdev->recv_buf); > + if (nvdev->recv_original_buf) > vfree(nvdev->recv_original_buf); > - } else { > + else > vfree(nvdev->recv_buf); > - } > > - if (nvdev->send_original_buf) { > - hv_unmap_memory(nvdev->send_buf); > + if (nvdev->send_original_buf) > vfree(nvdev->send_original_buf); > - } else { > + else > vfree(nvdev->send_buf); > - } > > bitmap_free(nvdev->send_section_map); > > @@ -765,6 +761,12 @@ void netvsc_device_remove(struct hv_device *device) > netvsc_teardown_send_gpadl(device, net_device, ndev); > } > > + if (net_device->recv_original_buf) > + hv_unmap_memory(net_device->recv_buf); > + > + if (net_device->send_original_buf) > + hv_unmap_memory(net_device->send_buf); > + > /* Release all resources */ > free_netvsc_device_rcu(net_device); > } > -- > 2.25.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-02-02 17:05 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-01 16:32 [PATCH] Netvsc: Call hv_unmap_memory() in the netvsc_device_remove() Tianyu Lan 2022-02-01 16:32 ` Tianyu Lan 2022-02-01 23:20 ` Haiyang Zhang 2022-02-01 23:20 ` Haiyang Zhang via iommu 2022-02-02 17:05 ` Michael Kelley (LINUX) [this message] 2022-02-02 17:05 ` Michael Kelley (LINUX) via iommu 2022-02-08 14:19 ` Tianyu Lan 2022-02-08 14:19 ` Tianyu Lan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=MWHPR21MB15935F58E55D05A171AE9ED4D7279@MWHPR21MB1593.namprd21.prod.outlook.com \ --to=mikelley@microsoft.com \ --cc=Tianyu.Lan@microsoft.com \ --cc=bp@alien8.de \ --cc=dave.hansen@linux.intel.com \ --cc=davem@davemloft.net \ --cc=decui@microsoft.com \ --cc=haiyangz@microsoft.com \ --cc=hch@infradead.org \ --cc=hpa@zytor.com \ --cc=iommu@lists.linux-foundation.org \ --cc=kuba@kernel.org \ --cc=kys@microsoft.com \ --cc=linux-hyperv@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ltykernel@gmail.com \ --cc=m.szyprowski@samsung.com \ --cc=mingo@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=robin.murphy@arm.com \ --cc=sthemmin@microsoft.com \ --cc=tglx@linutronix.de \ --cc=wei.liu@kernel.org \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.