On Tue, Apr 18, 2023 at 12:46:08AM +0800, Ming Lei wrote: > All ublk commands(control, IO) should have taken ioctl command encoding > from the beginning, because ioctl command encoding defines each code > uniquely, so driver can figure out wrong command sent from userspace easily; > 2) it might help security subsystem for audit uring cmd[1]. > > Unfortunately we didn't do that way, and it could be one lesson for ublk driver. FYI, a bunch of overly long lines in the commit message here, please stick to 73 lines. > > So switch to ioctl command encoding now, we still support commands encoded in > old way, but they become legacy definition. Any new command should take ioctl > encoding. > > ublksrv code for switching to ioctl command encoding: > > https://github.com/ming1/ubdsrv/commits/ioctl_cmd_encoding > > [1] https://lore.kernel.org/io-uring/CAHC9VhSVzujW9LOj5Km80AjU0EfAuukoLrxO6BEfnXeK_s6bAg@xxxxxxxxxxxxxx/ Maybe quote the main contents here? > +config BLK_DEV_UBLK_NO_OLD_CMD > + bool "Disable old command opcode" > + depends on BLK_DEV_UBLK > + default n I'd invert this option and have a BLKDEV_UBLK_LEGACY_OPCODES or so commands. (also n is the default default, so no need to state it, but with inverting the option that becomes moot anyway). > + if (IS_ENABLED(CONFIG_BLK_DEV_UBLK_NO_OLD_CMD) && ioc_type != 'u') > + return -EOPNOTSUPP; > + if (ioc_type != 'u' && ioc_type != 0) > + return -EOPNOTSUPP; Maybe switch these checks around for them to make more sense? > + if (IS_ENABLED(CONFIG_BLK_DEV_UBLK_NO_OLD_CMD) && ioc_type != 'u') > + return -EOPNOTSUPP; > + if (ioc_type != 'u' && ioc_type != 0) > + return -EOPNOTSUPP; And maybe factor this into a helper?
