scsi_transport_sas: fix BSG ioctl memory corruption

Commit Message

Omar Sandoval Feb. 21, 2017, 6:03 p.m. UTC

From: Omar Sandoval <[e-mail]>

The end_device and sas_host devices support BSG ioctls, but the
request_queue allocated for them isn't set up to allocate the struct
scsi_request payload. This leads to memory corruption in the call to
scsi_req_init() in bsg_map_hdr(), since it will memset past the end of
the allocated request. Fix it by setting ->cmd_size on the allocated
request_queue.

Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Signed-off-by: Omar Sandoval <[e-mail]>
---
I don't know what sg ioctls these devices actually support, but I tested
this with sg_inq, which previously caused KASAN splats.

 drivers/scsi/scsi_transport_sas.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Jens Axboe Feb. 21, 2017, 9:19 p.m. UTC | #1

On 02/21/2017 11:03 AM, Omar Sandoval wrote:
> From: Omar Sandoval <[e-mail]>
> 
> The end_device and sas_host devices support BSG ioctls, but the
> request_queue allocated for them isn't set up to allocate the struct
> scsi_request payload. This leads to memory corruption in the call to
> scsi_req_init() in bsg_map_hdr(), since it will memset past the end of
> the allocated request. Fix it by setting ->cmd_size on the allocated
> request_queue.

Good catch, thanks Omar. Applied.

Patch

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 126a5ee00987..f94535130a34 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -227,27 +227,31 @@  static int sas_bsg_initialize(struct Scsi_Host *shost, struct sas_rphy *rphy)
 		return 0;
 	}
 
+	q = blk_alloc_queue(GFP_KERNEL);
+	if (!q)
+		return -ENOMEM;
+	q->cmd_size = sizeof(struct scsi_request);
+
 	if (rphy) {
-		q = blk_init_queue(sas_non_host_smp_request, NULL);
+		q->request_fn = sas_non_host_smp_request;
 		dev = &rphy->dev;
 		name = dev_name(dev);
 		release = NULL;
 	} else {
-		q = blk_init_queue(sas_host_smp_request, NULL);
+		q->request_fn = sas_host_smp_request;
 		dev = &shost->shost_gendev;
 		snprintf(namebuf, sizeof(namebuf),
 			 "sas_host%d", shost->host_no);
 		name = namebuf;
 		release = sas_host_release;
 	}
-	if (!q)
-		return -ENOMEM;
+	error = blk_init_allocated_queue(q);
+	if (error)
+		goto out_cleanup_queue;
 
 	error = bsg_register_queue(q, dev, name, release);
-	if (error) {
-		blk_cleanup_queue(q);
-		return -ENOMEM;
-	}
+	if (error)
+		goto out_cleanup_queue;
 
 	if (rphy)
 		rphy->q = q;
@@ -261,6 +265,10 @@  static int sas_bsg_initialize(struct Scsi_Host *shost, struct sas_rphy *rphy)
 
 	queue_flag_set_unlocked(QUEUE_FLAG_BIDI, q);
 	return 0;
+
+out_cleanup_queue:
+	blk_cleanup_queue(q);
+	return error;
 }
 
 static void sas_bsg_remove(struct Scsi_Host *shost, struct sas_rphy *rphy)