Re: [PATCH v1 03/18] vfio/ccw: Ensure mdev->dev is cleared on mdev remove

Jason Gunthorpe
2022-06-03T13:37:21Z

On Fri, Jun 03, 2022 at 09:25:19AM -0400, Matthew Rosato wrote:
> On 6/2/22 1:19 PM, Eric Farman wrote:
> > The mdev is linked with the vfio_ccw_private pointer when the mdev
> > is probed, but it's not cleared once the mdev is removed.
> >
> > This isn't much of a concern based on the current device lifecycle,
> > but fix it so that things make sense in later shuffling.
> >
> > Fixes: 3bf1311f351ef ("vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()")
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > drivers/s390/cio/vfio_ccw_ops.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> > index a403d059a4e6..a0a3200b0b04 100644
> > +++ b/drivers/s390/cio/vfio_ccw_ops.c
> > @@ -159,6 +159,7 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
> > private->sch->schid.ssid,
> > private->sch->schid.sch_no);
> > + dev_set_drvdata(&mdev->dev, NULL);
> > vfio_unregister_group_dev(&private->vdev);
> > if ((private->state != VFIO_CCW_STATE_NOT_OPER) &&
>
> Seems harmless enough.
>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>
> But is this just precautionary or is it fixing a real problem (if the former
> I don't think a fixes tag makes sense)
>
> I also ask because I note vfio-ap clears its driver_data in mdev_remove but
> also leaves the pointer set, meaning they might need a similar cleanup and
> should probably have a look (CC Tony & Jason H)

There should be no reason to clear the drvdata on remove - the driver
must be designed to guarentee all references to the dev stop before
the remove function returns.

Jason