linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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, jasowang
  Cc: sgarzare, Max Gurtovoy, virtualization, linux-kernel, arei.gonglei



> 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, sgarzare, Max Gurtovoy, virtualization, linux-kernel,
	arei.gonglei

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, sgarzare, Max Gurtovoy, virtualization, linux-kernel,
	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;
as well as URLs for NNTP newsgroup(s).