Re: [PATCH v5 2/3] vfio/ism: Implement vfio_pci driver for ISM devices

Niklas Schnelle
2026-03-17T21:17:40Z

On Tue, 2026-03-17 at 11:43 -0700, Farhan Ali wrote:
> <..snip..>
>
> On 3/17/2026 5:58 AM, Julian Ruess wrote:
> > static int ism_vfio_pci_probe(struct pci_dev *pdev,
> > + const struct pci_device_id *id)
> > +{
> > + struct ism_vfio_pci_core_device *ivpcd;
> > + struct zpci_dev *zdev = to_zpci(pdev);
> > + char cache_name[20];
> > + int ret;
> > +
> > + ivpcd = vfio_alloc_device(ism_vfio_pci_core_device, core_device.vdev,
> > + &pdev->dev, &ism_pci_ops);
> > + if (IS_ERR(ivpcd))
> > + return PTR_ERR(ivpcd);
> > +
> > + snprintf(cache_name, sizeof(cache_name), "ism_sb_fid_%08x", zdev->fid);
> > + ivpcd->store_block_cache =
> > + kmem_cache_create(cache_name, zdev->maxstbl, 0, 0, NULL);
> > + if (!ivpcd->store_block_cache) {
> > + vfio_put_device(&ivpcd->core_device.vdev);
> > + return -ENOMEM;
> > + }
> > +
> > + dev_set_drvdata(&pdev->dev, &ivpcd->core_device);
> > + ret = vfio_pci_core_register_device(&ivpcd->core_device);
> > + if (ret) {
> > + kmem_cache_destroy(ivpcd->store_block_cache);
> > + vfio_put_device(&ivpcd->core_device.vdev);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void ism_vfio_pci_remove(struct pci_dev *pdev)
> > +{
> > + struct vfio_pci_core_device *core_device;
> > + struct ism_vfio_pci_core_device *ivpcd;
> > +
> > + core_device = dev_get_drvdata(&pdev->dev);
> > + ivpcd = container_of(core_device, struct ism_vfio_pci_core_device,
> > + core_device);
> > +
> > + vfio_pci_core_unregister_device(&ivpcd->core_device);
> > + vfio_put_device(&ivpcd->core_device.vdev);
> > +
> > + kmem_cache_destroy(ivpcd->store_block_cache);
>
> I think the kmem_cache_destroy() should be done before we do
> vfio_put_device() (or maybe even before
> vfio_pci_core_unregister_device()) to avoid use after free similar to
> ism_vfio_pci_probe(). Sorry I missed this earlier.
>
> Thanks
>
> Farhan
>

Good find! Wouldn't it be even cleaner to provide vfio/ism specific
.init and .release functions in ism_pci_ops that do the
kmem_cache_create() / kmem_cache_destroy() as part of init / release?
It seems this is done for e.g. Xe with xe_vfio_pci_init_dev()
and xe_vfio_pci_release_dev(). In fact wouldn't that be even necessary
to correctly handle the case where the above vfio_put_device() isn't
the last reference?

Thanks,
Niklas