On Thu, Jun 25, 2026 at 10:37 AM Xiong Weimin <[email protected]> wrote: > > From: Xiong Weimin <[email protected]> > > vdpasim_create() leaves vdpasim->worker as an ERR_PTR when > kthread_run_worker() fails. The error path then drops the device > reference, which releases the partially initialized simulator. > > vdpasim_free() unconditionally passes the worker pointer to > kthread_destroy_worker(), so the ERR_PTR is dereferenced and can > trigger a general protection fault. > > Store the worker error, clear the pointer, and make the release path > only clean up resources that were successfully initialized before > the failure. > > Tested on openEuler VM with kernel 6.16.8: module build, reload, > and vdpa dev add via vdpasim_net. > > Signed-off-by: Xiong Weimin <[email protected]> > --- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c > b/drivers/vdpa/vdpa_sim/vdpa_sim.c > index 1111111..2222222 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > @@ -231,8 +231,11 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr > *dev_attr, > kthread_init_work(&vdpasim->work, vdpasim_work_fn); > vdpasim->worker = kthread_run_worker(0, "vDPA sim worker: %s", > dev_attr->name); > - if (IS_ERR(vdpasim->worker)) > + if (IS_ERR(vdpasim->worker)) { > + ret = PTR_ERR(vdpasim->worker); > + vdpasim->worker = NULL; > goto err_iommu; > + } > > mutex_init(&vdpasim->mutex); > spin_lock_init(&vdpasim->iommu_lock); > @@ -750,18 +753,24 @@ static void vdpasim_free(struct vdpa_device *vdpa) > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > int i; > > - kthread_cancel_work_sync(&vdpasim->work); > - kthread_destroy_worker(vdpasim->worker); > + if (vdpasim->worker) { > + kthread_cancel_work_sync(&vdpasim->work); > + kthread_destroy_worker(vdpasim->worker); > + } > > - for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > - vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov); > - vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov); > + if (vdpasim->vqs) { > + for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > + vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov); > + vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov); > + } > } > > vdpasim->dev_attr.free(vdpasim); > > - for (i = 0; i < vdpasim->dev_attr.nas; i++) > - vhost_iotlb_reset(&vdpasim->iommu[i]); > + if (vdpasim->iommu && vdpasim->iommu_pt) { > + for (i = 0; i < vdpasim->dev_attr.nas; i++) > + vhost_iotlb_reset(&vdpasim->iommu[i]); > + } > kfree(vdpasim->iommu); > kfree(vdpasim->iommu_pt); > kfree(vdpasim->vqs);
Isn't this the exact same fix as v1 of https://lore.kernel.org/lkml/[email protected]/ ?
