linux-kernel - VLA removal, device_handler and COMMAND

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 9 Mar 2018 23:29:33 +0100
From:   Stephen Kitt <steve@....org>
To:     Jens Axboe <axboe@...nel.dk>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Hannes Reinecke <hare@...e.com>
Cc:     linux-block@...r.kernel.org, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: VLA removal, device_handler and COMMAND_SIZE

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

Content of type "application/pgp-signature" skipped