From: David Laight <David.Laight@ACULAB.COM>
To: 'Stephen Kitt' <steve@sk2.org>, "hare@suse.com" <hare@suse.com>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"jejb@linux.vnet.ibm.com" <jejb@linux.vnet.ibm.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: RE: [PATCH] scsi: resolve COMMAND_SIZE at compile time
Date: Tue, 13 Mar 2018 11:34:31 +0000 [thread overview]
Message-ID: <a66f8abb84444cf79b818011667bc825@AcuMS.aculab.com> (raw)
In-Reply-To: <20180309223355.21222-1-steve@sk2.org>
From: Stephen Kitt
> Sent: 09 March 2018 22:34
>
> COMMAND_SIZE currently uses an array of values in block/scsi_ioctl.c.
> A number of device_handler functions use this to initialise arrays,
> and this is flagged by -Wvla.
>
> This patch replaces COMMAND_SIZE with a variant using a formula which
> can be resolved at compile time in cases where the opcode is fixed,
> resolving the array size and avoiding the VLA. The downside is that
> the code is less maintainable and that some call sites end up having
> more complex generated code.
>
> Since scsi_command_size_tbl is dropped, we can remove the dependency
> on BLK_SCSI_REQUEST from drivers/target/Kconfig.
>
> This was prompted by https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> block/scsi_ioctl.c | 8 --------
> drivers/target/Kconfig | 1 -
> include/scsi/scsi_common.h | 13 +++++++++++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 60b471f8621b..b9e176e537be 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -41,14 +41,6 @@ struct blk_cmd_filter {
>
> static struct blk_cmd_filter blk_default_cmd_filter;
>
> -/* Command group 3 is reserved and should never be used. */
> -const unsigned char scsi_command_size_tbl[8] =
> -{
> - 6, 10, 10, 12,
> - 16, 12, 10, 10
> -};
> -EXPORT_SYMBOL(scsi_command_size_tbl);
> -
> #include <scsi/sg.h>
>
> static int sg_get_version(int __user *p)
> diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
> index 4c44d7bed01a..b5f05b60cf06 100644
> --- a/drivers/target/Kconfig
> +++ b/drivers/target/Kconfig
> @@ -4,7 +4,6 @@ menuconfig TARGET_CORE
> depends on SCSI && BLOCK
> select CONFIGFS_FS
> select CRC_T10DIF
> - select BLK_SCSI_REQUEST # only for scsi_command_size_tbl..
> select SGL_ALLOC
> default n
> help
> diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h
> index 731ac09ed231..48d950666376 100644
> --- a/include/scsi/scsi_common.h
> +++ b/include/scsi/scsi_common.h
> @@ -15,8 +15,17 @@ scsi_varlen_cdb_length(const void *hdr)
> return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8;
> }
>
> -extern const unsigned char scsi_command_size_tbl[8];
> -#define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7]
> +/*
> + * SCSI command sizes are as follows, in bytes, for fixed size commands, per
> + * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode
> + * determine its group.
> + * The size table is encoded into a 32-bit value by subtracting each value
> + * from 16, resulting in a value of 1715488362
> + * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 + 10).
> + * Command group 3 is reserved and should never be used.
> + */
> +#define COMMAND_SIZE(opcode) \
> + (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7)))))
Why not:
(6 + (15 & (0x446a6440u ....
Specifying the constant in hex makes it more obvious.
It is a shame about the 16, but an offset is easier than the negate.
David
prev parent reply other threads:[~2018-03-13 11:34 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-09 22:29 VLA removal, device_handler and COMMAND_SIZE Stephen Kitt
2018-03-09 22:32 ` [PATCH] device_handler: remove VLAs Stephen Kitt
2018-03-09 22:48 ` Bart Van Assche
2018-03-10 13:14 ` Stephen Kitt
2018-03-12 15:41 ` Bart Van Assche
2018-03-12 19:26 ` Stephen Kitt
2018-03-12 6:25 ` Hannes Reinecke
2018-03-13 2:37 ` Martin K. Petersen
2018-03-09 22:33 ` [PATCH] scsi: resolve COMMAND_SIZE at compile time Stephen Kitt
2018-03-09 22:47 ` Bart Van Assche
2018-03-10 13:29 ` Stephen Kitt
2018-03-10 20:49 ` James Bottomley
2018-03-10 21:16 ` Douglas Gilbert
2018-03-13 11:34 ` David Laight [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a66f8abb84444cf79b818011667bc825@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=axboe@kernel.dk \
--cc=hare@suse.com \
--cc=jejb@linux.vnet.ibm.com \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=steve@sk2.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).