Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The UAPI ioctl command macro for MSHV_CREATE_DEVICE is missing. - [Medium] The ops structure mshv_vfio_device_ops is not declared as const. --
--- Patch [2]: [PATCH V4 2/9] mshv: Add declarations and definitions for VFIO-MSHV bridge device --- commit dbad27877cef927b38632a1b77ab492ceeebae34 Author: Mukesh R <[email protected]> mshv: Add declarations and definitions for VFIO-MSHV bridge device Add data structs needed by the subsequent patch that introduces a new module to implement VFIO-MSHV pseudo device. > diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h > index a85c24dcc701a..b9880d0bdc4df 100644 > --- a/drivers/hv/mshv_root.h > +++ b/drivers/hv/mshv_root.h [ ... ] > +struct mshv_device_ops { > + const char *device_name; > + long (*device_create)(struct mshv_device *dev); > + void (*device_release)(struct mshv_device *dev); > + long (*device_set_attr)(struct mshv_device *dev, > + struct mshv_device_attr *attr); > + long (*device_has_attr)(struct mshv_device *dev, > + struct mshv_device_attr *attr); > +}; > + > +extern struct mshv_device_ops mshv_vfio_device_ops; [Severity: Medium] Should mshv_vfio_device_ops be declared as const? Operations structures containing function pointers are typically placed in read-only memory as a security hardening measure. Missing the const keyword on the extern declaration prevents the future definition from being const without triggering compilation errors. > diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h > index 32ff92b6342b2..be6fe3ee87078 100644 > --- a/include/uapi/linux/mshv.h > +++ b/include/uapi/linux/mshv.h [ ... ] > +/* Device passhthru */ > +#define MSHV_CREATE_DEVICE_TEST 1 > + > +enum { > + MSHV_DEV_TYPE_VFIO, > + MSHV_DEV_TYPE_MAX, > +}; > + > +struct mshv_create_device { > + __u32 type; /* in: MSHV_DEV_TYPE_xxx */ > + __u32 fd; /* out: device handle */ > + __u32 flags; /* in: MSHV_CREATE_DEVICE_xxx */ > +}; > + > +#define MSHV_DEV_VFIO_FILE 1 > +#define MSHV_DEV_VFIO_FILE_ADD 1 > +#define MSHV_DEV_VFIO_FILE_DEL 2 > + > +struct mshv_device_attr { > + __u32 flags; /* no flags currently defined */ > + __u32 group; /* device-defined */ > + __u64 attr; /* group-defined */ > + __u64 addr; /* userspace address of attr data */ > +}; > + > +/* Device fds created with MSHV_CREATE_DEVICE */ > +#define MSHV_SET_DEVICE_ATTR _IOW(MSHV_IOCTL, 0x00, struct mshv_device_attr) > +#define MSHV_HAS_DEVICE_ATTR _IOW(MSHV_IOCTL, 0x01, struct mshv_device_attr) [Severity: High] Is the UAPI ioctl command macro for MSHV_CREATE_DEVICE missing from this header? The struct mshv_create_device is defined for the ioctl payload, and the comments reference MSHV_CREATE_DEVICE, but the actual ioctl command macro appears to be absent. Without this definition, userspace cannot compile or invoke the ioctl to create a device. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
