Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unprotected access to `pt_devices` list and missing lockdep assertion. - [Low] The `mshv_vfio_device_ops` structure is missing the `const` qualifier. --
commit 96ab470ecc51b095ed8b1538e4abdf37d030b039 Author: Mukesh R <[email protected]> Subject: mshv: Implement mshv bridge device for VFIO This commit introduces a new VFIO-MSHV bridge pseudo device file, adapted from KVM's VFIO bridge. It provides the necessary device operations to link VFIO files with an MSHV partition. > diff --git a/drivers/hv/mshv_vfio.c b/drivers/hv/mshv_vfio.c > new file mode 100644 > index 0000000000000..00a97920e25ba > --- /dev/null > +++ b/drivers/hv/mshv_vfio.c [ ... ] > +static long mshv_vfio_create_device(struct mshv_device *mshvdev) > +{ > + struct mshv_device *tmp; > + struct mshv_vfio *mshv_vfio; > + > + /* Only one VFIO "device" per VM */ > + hlist_for_each_entry(tmp, &mshvdev->device_pt->pt_devices, > + device_ptnode) > + if (tmp->device_ops == &mshv_vfio_device_ops) > + return -EBUSY; Are we missing a lockdep assertion here to ensure the partition lock is held? Since this iterates over the pt_devices list to enforce a singleton device, could concurrent ioctl calls bypass this check and corrupt the hlist if the caller doesn't acquire the lock? [ ... ] > +struct mshv_device_ops mshv_vfio_device_ops = { > + .device_name = "mshv-vfio", > + .device_create = mshv_vfio_create_device, > + .device_release = mshv_vfio_release_device, > + .device_set_attr = mshv_vfio_set_attr, > + .device_has_attr = mshv_vfio_has_attr, > +}; Should mshv_vfio_device_ops be declared as const? The structure consists entirely of function pointers and is never modified after initialization. Making it const would place it in read-only memory. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
