* [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
@ 2021-11-22 12:22 Longpeng(Mike)
2021-11-23 2:10 ` Jason Wang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Longpeng(Mike) @ 2021-11-22 12:22 UTC (permalink / raw)
To: mst, jasowang
Cc: sgarzare, mgurtovoy, parav, virtualization, linux-kernel,
arei.gonglei, Longpeng
From: Longpeng <longpeng2@huawei.com>
The system will crash if we put an uninitialized iova_domain, this
could happen when an error occurs before initializing the iova_domain
in vdpasim_create().
BUG: kernel NULL pointer dereference, address: 0000000000000000
...
RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
...
Call Trace:
<TASK>
put_iova_domain+0x29/0x220
vdpasim_free+0xd1/0x120 [vdpa_sim]
vdpa_release_dev+0x21/0x40 [vdpa]
device_release+0x33/0x90
kobject_release+0x63/0x160
vdpasim_create+0x127/0x2a0 [vdpa_sim]
vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
genl_family_rcv_msg_doit+0x112/0x140
genl_rcv_msg+0xdf/0x1d0
...
So we must make sure the iova_domain is already initialized before
put it.
In addition, we may get the following warning in this case:
WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
So we must make sure the iova_cache_put() is invoked only if the
iova_cache_get() is already invoked. Let's fix it together.
Signed-off-by: Longpeng <longpeng2@huawei.com>
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 5f484fff8dbe..41b0cd17fcba 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -591,8 +591,11 @@ static void vdpasim_free(struct vdpa_device *vdpa)
vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
}
- put_iova_domain(&vdpasim->iova);
- iova_cache_put();
+ if (vdpa_get_dma_dev(vdpa)) {
+ put_iova_domain(&vdpasim->iova);
+ iova_cache_put();
+ }
+
kvfree(vdpasim->buffer);
if (vdpasim->iommu)
vhost_iotlb_free(vdpasim->iommu);
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
2021-11-22 12:22 [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain Longpeng(Mike)
@ 2021-11-23 2:10 ` Jason Wang
2021-11-23 3:12 ` Parav Pandit
2021-11-23 8:19 ` Stefano Garzarella
2 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2021-11-23 2:10 UTC (permalink / raw)
To: Longpeng(Mike)
Cc: mst, Stefano Garzarella, Max Gurtovoy, Parav Pandit,
virtualization, linux-kernel, Gonglei (Arei)
On Mon, Nov 22, 2021 at 8:22 PM Longpeng(Mike) <longpeng2@huawei.com> wrote:
>
> From: Longpeng <longpeng2@huawei.com>
>
> The system will crash if we put an uninitialized iova_domain, this
> could happen when an error occurs before initializing the iova_domain
> in vdpasim_create().
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> ...
> RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
> ...
> Call Trace:
> <TASK>
> put_iova_domain+0x29/0x220
> vdpasim_free+0xd1/0x120 [vdpa_sim]
> vdpa_release_dev+0x21/0x40 [vdpa]
> device_release+0x33/0x90
> kobject_release+0x63/0x160
> vdpasim_create+0x127/0x2a0 [vdpa_sim]
> vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
> vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
> genl_family_rcv_msg_doit+0x112/0x140
> genl_rcv_msg+0xdf/0x1d0
> ...
>
> So we must make sure the iova_domain is already initialized before
> put it.
>
> In addition, we may get the following warning in this case:
> WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
>
> So we must make sure the iova_cache_put() is invoked only if the
> iova_cache_get() is already invoked. Let's fix it together.
>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
Acked-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 5f484fff8dbe..41b0cd17fcba 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -591,8 +591,11 @@ static void vdpasim_free(struct vdpa_device *vdpa)
> vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
> }
>
> - put_iova_domain(&vdpasim->iova);
> - iova_cache_put();
> + if (vdpa_get_dma_dev(vdpa)) {
> + put_iova_domain(&vdpasim->iova);
> + iova_cache_put();
> + }
> +
> kvfree(vdpasim->buffer);
> if (vdpasim->iommu)
> vhost_iotlb_free(vdpasim->iommu);
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
2021-11-22 12:22 [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain Longpeng(Mike)
2021-11-23 2:10 ` Jason Wang
@ 2021-11-23 3:12 ` Parav Pandit
2021-11-23 4:12 ` Jason Wang
2021-11-23 8:19 ` Stefano Garzarella
2 siblings, 1 reply; 6+ messages in thread
From: Parav Pandit @ 2021-11-23 3:12 UTC (permalink / raw)
To: Longpeng(Mike), mst@redhat.com, jasowang@redhat.com
Cc: sgarzare@redhat.com, Max Gurtovoy,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, arei.gonglei@huawei.com
> From: Longpeng(Mike) <longpeng2@huawei.com>
> Sent: Monday, November 22, 2021 5:52 PM
>
> From: Longpeng <longpeng2@huawei.com>
>
> The system will crash if we put an uninitialized iova_domain, this could
> happen when an error occurs before initializing the iova_domain in
> vdpasim_create().
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000 ...
> RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
> ...
> Call Trace:
> <TASK>
> put_iova_domain+0x29/0x220
> vdpasim_free+0xd1/0x120 [vdpa_sim]
> vdpa_release_dev+0x21/0x40 [vdpa]
> device_release+0x33/0x90
> kobject_release+0x63/0x160
> vdpasim_create+0x127/0x2a0 [vdpa_sim]
> vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
> vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
> genl_family_rcv_msg_doit+0x112/0x140
> genl_rcv_msg+0xdf/0x1d0
> ...
>
> So we must make sure the iova_domain is already initialized before put it.
>
> In addition, we may get the following warning in this case:
> WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
>
> So we must make sure the iova_cache_put() is invoked only if the
> iova_cache_get() is already invoked. Let's fix it together.
>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
Can you please add the fixes tag here so that older kernels can take this fix?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
2021-11-23 3:12 ` Parav Pandit
@ 2021-11-23 4:12 ` Jason Wang
2021-11-23 6:12 ` Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2021-11-23 4:12 UTC (permalink / raw)
To: Parav Pandit
Cc: Longpeng(Mike), mst@redhat.com, sgarzare@redhat.com, Max Gurtovoy,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, arei.gonglei@huawei.com
On Tue, Nov 23, 2021 at 11:12 AM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> > From: Longpeng(Mike) <longpeng2@huawei.com>
> > Sent: Monday, November 22, 2021 5:52 PM
> >
> > From: Longpeng <longpeng2@huawei.com>
> >
> > The system will crash if we put an uninitialized iova_domain, this could
> > happen when an error occurs before initializing the iova_domain in
> > vdpasim_create().
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000000 ...
> > RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
> > ...
> > Call Trace:
> > <TASK>
> > put_iova_domain+0x29/0x220
> > vdpasim_free+0xd1/0x120 [vdpa_sim]
> > vdpa_release_dev+0x21/0x40 [vdpa]
> > device_release+0x33/0x90
> > kobject_release+0x63/0x160
> > vdpasim_create+0x127/0x2a0 [vdpa_sim]
> > vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
> > vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
> > genl_family_rcv_msg_doit+0x112/0x140
> > genl_rcv_msg+0xdf/0x1d0
> > ...
> >
> > So we must make sure the iova_domain is already initialized before put it.
> >
> > In addition, we may get the following warning in this case:
> > WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
> >
> > So we must make sure the iova_cache_put() is invoked only if the
> > iova_cache_get() is already invoked. Let's fix it together.
> >
> > Signed-off-by: Longpeng <longpeng2@huawei.com>
>
> Can you please add the fixes tag here so that older kernels can take this fix?
>
I guess it's 4080fc106750 ("vdpa_sim: use iova module to allocate IOVA
addresses")
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
2021-11-23 4:12 ` Jason Wang
@ 2021-11-23 6:12 ` Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
0 siblings, 0 replies; 6+ messages in thread
From: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) @ 2021-11-23 6:12 UTC (permalink / raw)
To: Jason Wang, Parav Pandit
Cc: mst@redhat.com, sgarzare@redhat.com, Max Gurtovoy,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, Gonglei (Arei)
> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Tuesday, November 23, 2021 12:13 PM
> To: Parav Pandit <parav@nvidia.com>
> Cc: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@huawei.com>; mst@redhat.com; sgarzare@redhat.com; Max Gurtovoy
> <mgurtovoy@nvidia.com>; virtualization@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Gonglei (Arei) <arei.gonglei@huawei.com>
> Subject: Re: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
>
> On Tue, Nov 23, 2021 at 11:12 AM Parav Pandit <parav@nvidia.com> wrote:
> >
> >
> >
> > > From: Longpeng(Mike) <longpeng2@huawei.com>
> > > Sent: Monday, November 22, 2021 5:52 PM
> > >
> > > From: Longpeng <longpeng2@huawei.com>
> > >
> > > The system will crash if we put an uninitialized iova_domain, this could
> > > happen when an error occurs before initializing the iova_domain in
> > > vdpasim_create().
> > >
> > > BUG: kernel NULL pointer dereference, address: 0000000000000000 ...
> > > RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
> > > ...
> > > Call Trace:
> > > <TASK>
> > > put_iova_domain+0x29/0x220
> > > vdpasim_free+0xd1/0x120 [vdpa_sim]
> > > vdpa_release_dev+0x21/0x40 [vdpa]
> > > device_release+0x33/0x90
> > > kobject_release+0x63/0x160
> > > vdpasim_create+0x127/0x2a0 [vdpa_sim]
> > > vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
> > > vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
> > > genl_family_rcv_msg_doit+0x112/0x140
> > > genl_rcv_msg+0xdf/0x1d0
> > > ...
> > >
> > > So we must make sure the iova_domain is already initialized before put it.
> > >
> > > In addition, we may get the following warning in this case:
> > > WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
> > >
> > > So we must make sure the iova_cache_put() is invoked only if the
> > > iova_cache_get() is already invoked. Let's fix it together.
> > >
> > > Signed-off-by: Longpeng <longpeng2@huawei.com>
> >
> > Can you please add the fixes tag here so that older kernels can take this fix?
> >
>
> I guess it's 4080fc106750 ("vdpa_sim: use iova module to allocate IOVA
> addresses")
>
I think so. I'll add the fixes tag in V2, thanks.
> Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain
2021-11-22 12:22 [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain Longpeng(Mike)
2021-11-23 2:10 ` Jason Wang
2021-11-23 3:12 ` Parav Pandit
@ 2021-11-23 8:19 ` Stefano Garzarella
2 siblings, 0 replies; 6+ messages in thread
From: Stefano Garzarella @ 2021-11-23 8:19 UTC (permalink / raw)
To: Longpeng(Mike)
Cc: mst, jasowang, mgurtovoy, parav, virtualization, linux-kernel,
arei.gonglei
On Mon, Nov 22, 2021 at 08:22:21PM +0800, Longpeng(Mike) wrote:
>From: Longpeng <longpeng2@huawei.com>
>
>The system will crash if we put an uninitialized iova_domain, this
>could happen when an error occurs before initializing the iova_domain
>in vdpasim_create().
>
>BUG: kernel NULL pointer dereference, address: 0000000000000000
>...
>RIP: 0010:__cpuhp_state_remove_instance+0x96/0x1c0
>...
>Call Trace:
> <TASK>
> put_iova_domain+0x29/0x220
> vdpasim_free+0xd1/0x120 [vdpa_sim]
> vdpa_release_dev+0x21/0x40 [vdpa]
> device_release+0x33/0x90
> kobject_release+0x63/0x160
> vdpasim_create+0x127/0x2a0 [vdpa_sim]
> vdpasim_net_dev_add+0x7d/0xfe [vdpa_sim_net]
> vdpa_nl_cmd_dev_add_set_doit+0xe1/0x1a0 [vdpa]
> genl_family_rcv_msg_doit+0x112/0x140
> genl_rcv_msg+0xdf/0x1d0
> ...
>
>So we must make sure the iova_domain is already initialized before
>put it.
>
>In addition, we may get the following warning in this case:
>WARNING: ... drivers/iommu/iova.c:344 iova_cache_put+0x58/0x70
>
>So we must make sure the iova_cache_put() is invoked only if the
>iova_cache_get() is already invoked. Let's fix it together.
>
>Signed-off-by: Longpeng <longpeng2@huawei.com>
>---
> drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Ooops, thanks for fixing this :-)
With the Fixes tag as suggested:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-23 8:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-22 12:22 [PATCH] vdpa_sim: avoid putting an uninitialized iova_domain Longpeng(Mike)
2021-11-23 2:10 ` Jason Wang
2021-11-23 3:12 ` Parav Pandit
2021-11-23 4:12 ` Jason Wang
2021-11-23 6:12 ` Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
2021-11-23 8:19 ` Stefano Garzarella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox