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 0fd79b7040b09fc840e0499afbc88e68b34a3e18 Author: Christian Brauner <[email protected]> scsi: esas2r: let procfs copy the ioctl argument of the ATTONode file This commit factors the core logic of esas2r_ioctl_handler() into esas2r_ioctl_buf() operating on a kernel copy of struct atto_express_ioctl, and converts the procfs ATTONode file to use .proc_ioctl_buf and .proc_ioctl_args so procfs handles copying the ioctl buffer in and out. > diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c > b/drivers/scsi/esas2r/esas2r_ioctl.c > index f99318f61c934..eb39ba8263b31 100644 > --- a/drivers/scsi/esas2r/esas2r_ioctl.c > +++ b/drivers/scsi/esas2r/esas2r_ioctl.c [ ... ] > @@ -1515,19 +1501,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] This isn't a bug, but in esas2r_ioctl_handler() in drivers/scsi/esas2r/esas2r_ioctl.c, ioctl is declared with __free(kfree) = NULL at the top of the function before the argument validation check and assigned later via memdup_user(). Per the Cleanup API conventions, could the __free(kfree) variable definition and initialization be combined into a single statement at the point of allocation: struct atto_express_ioctl *ioctl __free(kfree) = memdup_user(arg, sizeof(struct atto_express_ioctl)); after the early return check? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=14
