Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] scsi: esas2r: split declaration and initialization of __free() variable in esas2r_ioctl_handler() --
commit 6197b49c6d17310202283a54ea188828c65f914a Author: Christian Brauner <[email protected]> scsi: esas2r: let procfs copy the ioctl argument of the ATTONode file This commit factors the core ATTO ioctl handling out of esas2r_ioctl_handler() into esas2r_ioctl_buf() and wires up the procfs entry to use proc_ioctl_buf with a table of supported commands so procfs handles copying the ioctl buffer to and from userspace. > diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c > b/drivers/scsi/esas2r/esas2r_ioctl.c > index 80ae5b9d45db1..901c5d40cda8c 100644 > --- a/drivers/scsi/esas2r/esas2r_ioctl.c > +++ b/drivers/scsi/esas2r/esas2r_ioctl.c [ ... ] > @@ -1516,19 +1502,43 @@ int esas2r_ioctl_handler(void *hostdata, unsigned int > cmd, void __user *arg) > > } > > + return 0; > +} > + > +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg) > +{ > + struct atto_express_ioctl *ioctl __free(kfree) = NULL; > + int err; > + > + esas2r_log(ESAS2R_LOG_DEBG, "ioctl (%p, %x, %p)", hostdata, cmd, arg); > + > + if ((arg == NULL) > + || (cmd < EXPRESS_IOCTL_MIN) > + || (cmd > EXPRESS_IOCTL_MAX)) > + return -ENOTSUPP; > + > + ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl)); [Severity: Low] As reported by Sashiko on RFC [1], this isn't a bug, but in esas2r_ioctl_handler() in drivers/scsi/esas2r/esas2r_ioctl.c, the ioctl pointer is declared with __free(kfree) = NULL at the top of the function and then assigned via memdup_user() after the early validation check. Per the cleanup.h conventions, could ioctl be defined and initialized in a single statement at the memdup_user() call site instead of declaring it at the top of the function? [1] https://lore.kernel.org/r/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
