vhost: initialize vq->nheads properly

[PATCH] vhost: initialize vq->nheads properly

Posted by Jason Wang 6 months, 1 week ago

Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
vq->nheads to store the number of batched used buffers per used elem
but it forgets to initialize the vq->nheads to NULL in
vhost_dev_init() this will cause kfree() that would try to free it
without be allocated if SET_OWNER is not called.

Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
Reported-by: Breno Leitao <leitao@debian.org>
Fixes: 7918bb2d19c9 ("vhost: basic in order support")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index a4873d116df1..b4dfe38c7008 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -615,6 +615,7 @@ void vhost_dev_init(struct vhost_dev *dev,
 		vq->log = NULL;
 		vq->indirect = NULL;
 		vq->heads = NULL;
+		vq->nheads = NULL;
 		vq->dev = dev;
 		mutex_init(&vq->mutex);
 		vhost_vq_reset(dev, vq);
-- 
2.39.5

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by Jason Wang 6 months, 1 week ago

Hi Michael:

On Tue, Jul 29, 2025 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote:
>
> Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
> vq->nheads to store the number of batched used buffers per used elem
> but it forgets to initialize the vq->nheads to NULL in
> vhost_dev_init() this will cause kfree() that would try to free it
> without be allocated if SET_OWNER is not called.
>
> Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
> Reported-by: Breno Leitao <leitao@debian.org>
> Fixes: 7918bb2d19c9 ("vhost: basic in order support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I didn't see this in your pull request.

Thanks

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by Michael S. Tsirkin 6 months, 1 week ago

On Mon, Aug 04, 2025 at 05:05:57PM +0800, Jason Wang wrote:
> Hi Michael:
> 
> On Tue, Jul 29, 2025 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
> > vq->nheads to store the number of batched used buffers per used elem
> > but it forgets to initialize the vq->nheads to NULL in
> > vhost_dev_init() this will cause kfree() that would try to free it
> > without be allocated if SET_OWNER is not called.
> >
> > Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
> > Reported-by: Breno Leitao <leitao@debian.org>
> > Fixes: 7918bb2d19c9 ("vhost: basic in order support")
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> 
> I didn't see this in your pull request.
> 
> Thanks

in next now. Will be in the next pull, thanks!

-- 
MST

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by JAEHOON KIM 6 months, 1 week ago

On 7/29/2025 2:39 AM, Jason Wang wrote:
> Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
> vq->nheads to store the number of batched used buffers per used elem
> but it forgets to initialize the vq->nheads to NULL in
> vhost_dev_init() this will cause kfree() that would try to free it
> without be allocated if SET_OWNER is not called.
>
> Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
> Reported-by: Breno Leitao <leitao@debian.org>
> Fixes: 7918bb2d19c9 ("vhost: basic in order support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>   drivers/vhost/vhost.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a4873d116df1..b4dfe38c7008 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -615,6 +615,7 @@ void vhost_dev_init(struct vhost_dev *dev,
>   		vq->log = NULL;
>   		vq->indirect = NULL;
>   		vq->heads = NULL;
> +		vq->nheads = NULL;
>   		vq->dev = dev;
>   		mutex_init(&vq->mutex);
>   		vhost_vq_reset(dev, vq);
>
checked and confirmed no crash occurs.
Thanks for the fast update.

Tested-by: Jaehoon Kim <jhkim@linux.ibm.com>

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by Stefano Garzarella 6 months, 1 week ago

On Tue, Jul 29, 2025 at 03:39:16PM +0800, Jason Wang wrote:
>Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
>vq->nheads to store the number of batched used buffers per used elem
>but it forgets to initialize the vq->nheads to NULL in
>vhost_dev_init() this will cause kfree() that would try to free it
>without be allocated if SET_OWNER is not called.
>
>Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
>Reported-by: Breno Leitao <leitao@debian.org>
>Fixes: 7918bb2d19c9 ("vhost: basic in order support")
>Signed-off-by: Jason Wang <jasowang@redhat.com>
>---
> drivers/vhost/vhost.c | 1 +
> 1 file changed, 1 insertion(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index a4873d116df1..b4dfe38c7008 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -615,6 +615,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> 		vq->log = NULL;
> 		vq->indirect = NULL;
> 		vq->heads = NULL;
>+		vq->nheads = NULL;
> 		vq->dev = dev;
> 		mutex_init(&vq->mutex);
> 		vhost_vq_reset(dev, vq);
>-- 
>2.39.5
>

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by Breno Leitao 6 months, 1 week ago

On Tue, Jul 29, 2025 at 03:39:16PM +0800, Jason Wang wrote:
> Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
> vq->nheads to store the number of batched used buffers per used elem
> but it forgets to initialize the vq->nheads to NULL in
> vhost_dev_init() this will cause kfree() that would try to free it
> without be allocated if SET_OWNER is not called.
> 
> Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
> Reported-by: Breno Leitao <leitao@debian.org>
> Fixes: 7918bb2d19c9 ("vhost: basic in order support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Tested-by: Breno Leitao <leitao@debian.org>

Re: [PATCH] vhost: initialize vq->nheads properly

Posted by Dawid Osuchowski 6 months, 1 week ago

On 2025-07-29 9:39 AM, Jason Wang wrote:
> Commit 7918bb2d19c9 ("vhost: basic in order support") introduces
> vq->nheads to store the number of batched used buffers per used elem
> but it forgets to initialize the vq->nheads to NULL in
> vhost_dev_init() this will cause kfree() that would try to free it
> without be allocated if SET_OWNER is not called.

nit: as someone who is not familiar with vhost code, it took me a while 
to figure out you meant VHOST_SET_OWNER and the corresponding 
vhost_dev_set_owner()

> 
> Reported-by: JAEHOON KIM <jhkim@linux.ibm.com>
> Reported-by: Breno Leitao <leitao@debian.org>
> Fixes: 7918bb2d19c9 ("vhost: basic in order support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Reviewed-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

Thanks,
Dawid