[02/18] bsg: Check private request size before attaching to a queue

Commit Message

Bart Van Assche May 19, 2017, 6:30 p.m. UTC

Since BSG only supports request queues for which struct scsi_request
is the first member of their private request data, refuse to register
block layer queues for which the private data is smaller than struct
scsi_request.

References: commit bd1599d931ca ("scsi_transport_sas: fix BSG ioctl memory corruption")
References: commit 82ed4db499b8 ("block: split scsi_request out of struct request")
Signed-off-by: Bart Van Assche <[e-mail]>
Cc: Christoph Hellwig <[e-mail]>
Cc: Omar Sandoval <[e-mail]>
Cc: Hannes Reinecke <[e-mail]>
Cc: [e-mail]
---
 block/bsg.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Christoph Hellwig May 21, 2017, 6:32 a.m. UTC | #1

Hi Bart,

I think this is the wrong kind of check - while we do care about the
size of the queue, we only do it as a side effect of the queue
being able to handle REQ_OP_SCSI_IN/REQ_OP_SCSI_OUT commands.

I think we'll need a flag for those in the queue instead.

And btw, I didn't get your cover letter [0/18], did that get lost
somewhere?

Bart Van Assche May 21, 2017, 2:33 p.m. UTC | #2

On Sun, 2017-05-21 at 08:32 +0200, Christoph Hellwig wrote:
> And btw, I didn't get your cover letter [0/18], did that get lost

> somewhere?


Hello Christoph,

Thanks for the review comments. The cover letter should have made it to at
least the linux-scsi mailing list since it shows up in at least one archive of
that mailing list: https://www.spinics.net/lists/linux-scsi/msg108940.html.

Bart.

Christoph Hellwig May 22, 2017, 7:49 a.m. UTC | #3

On Sun, May 21, 2017 at 02:33:05PM +0000, Bart Van Assche wrote:
> Thanks for the review comments. The cover letter should have made it to at
> least the linux-scsi mailing list since it shows up in at least one archive of
> that mailing list: https://www.spinics.net/lists/linux-scsi/msg108940.html.

Yes, I see it on the list now.  But it's missing various Cc that the
actual patches have, including that to me, which seems a bit broken.

Patch

diff --git a/block/bsg.c b/block/bsg.c
index 6fd08544d77e..5ccecc9855ac 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -750,6 +750,12 @@  static struct bsg_device *bsg_add_device(struct inode *inode,
 #ifdef BSG_DEBUG
 	unsigned char buf[32];
 #endif
+
+	if (blk_queue_cmd_size(rq) < sizeof(struct scsi_request)) {
+		WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
+		return ERR_PTR(-EINVAL);
+	}
+
 	if (!blk_get_queue(rq))
 		return ERR_PTR(-ENXIO);