All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Kitt <steve@sk2.org>
To: Jens Axboe <axboe@kernel.dk>,
	"James E.J. Bottomley" <jejb@linux.vnet.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Hannes Reinecke <hare@suse.com>
Cc: linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Kernel Hardening <kernel-hardening@lists.openwall.com>
Subject: VLA removal, device_handler and COMMAND_SIZE
Date: Fri, 9 Mar 2018 23:29:33 +0100	[thread overview]
Message-ID: <20180309232933.14e39858@heffalump.sk2.org> (raw)

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

Hi,

I’ve been looking into removing some VLAs from device_handler drivers,
prompted by https://lkml.org/lkml/2018/3/7/621

The uses in question here are quite straightforward, e.g. in
drivers/scsi/device_handler/scsi_dh_alua.c:

	u8 cdb[COMMAND_SIZE(MAINTENANCE_IN)];

There’s no trivial way of keeping the compiler happy with -Wvla though here,
at least not while keeping the behaviour strictly identical. I’ve come up
with two approaches, and I’m curious whether they’re appropriate or if
there’s a better way...

The first approach is to use MAX_COMMAND_SIZE instead; this wastes a few
bytes on the stack here and there, and stays reasonably maintainable.

The second approach might be symptomatic of a twisted mind, and involves
replacing COMMAND_SIZE so that it can be calculated at compile time when the
opcode is known:

/*
 * 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)))))


This has the side-effect of making some of the call sites more complex, and
the macro itself isn’t the most maintainer-friendly. It does mean we can drop
BLK_SCSI_REQUEST from drivers/target/Kconfig so it’s not all bad...

Both patches will follow in reply to this email, I’ll let more familiar
developers judge which is appropriate (if any).

Regards,

Stephen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

             reply	other threads:[~2018-03-09 22:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-09 22:29 Stephen Kitt [this message]
2018-03-09 22:32 ` [PATCH] device_handler: remove VLAs Stephen Kitt
2018-03-09 22:48   ` Bart Van Assche
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 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-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-11 15:01         ` Stephen Kitt
2018-03-11 15:01           ` Stephen Kitt
2018-03-13 11:34   ` David Laight
2018-03-13 11:34     ` David Laight

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=20180309232933.14e39858@heffalump.sk2.org \
    --to=steve@sk2.org \
    --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 \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.