4.10 and -stable fix for virtio_blk and virtually mapped stacks

7 messages, 5 authors, 2017-01-09 · open the first message on its own page

4.10 and -stable fix for virtio_blk and virtually mapped stacks

From: Christoph Hellwig <hch@lst.de>
Date: 2017-01-04 05:25:17 

Without this fix attempts to do scsi passthrough on virtio_blk will crash
the system on virtually mapped stacks, which is something happening during
boot with many distros.

[PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: Christoph Hellwig <hch@lst.de>
Date: 2017-01-04 05:25:25 

Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/virtio_blk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
 	struct virtio_blk_outhdr out_hdr;
 	struct virtio_scsi_inhdr in_hdr;
 	u8 status;
+	u8 sense[SCSI_SENSE_BUFFERSIZE];
 	struct scatterlist sg[];
 };
 
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
 	}
 
 	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
 		sgs[num_out + num_in++] = &sense;
 		sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
 		sgs[num_out + num_in++] = &inhdr;
-- 
2.1.4

Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: Jason Wang <hidden>
Date: 2017-01-04 07:45:35 


On 2017年01月04日 13:25, Christoph Hellwig wrote:
quoted hunk
Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
  drivers/block/virtio_blk.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
  	struct virtio_blk_outhdr out_hdr;
  	struct virtio_scsi_inhdr in_hdr;
  	u8 status;
+	u8 sense[SCSI_SENSE_BUFFERSIZE];
  	struct scatterlist sg[];
  };
  
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
  	}
  
  	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
  		sgs[num_out + num_in++] = &sense;
  		sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
  		sgs[num_out + num_in++] = &inhdr;
Acked-by: Jason Wang <redacted>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: 王金浦 <hidden>
Date: 2017-01-04 15:47:03 

Hi Christoph,

2017-01-04 6:25 GMT+01:00 Christoph Hellwig [off-list ref]:
Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.
This sounds scary.
Could you share how to reproduce it, this should go into stable if
it's the case.

Thanks,
Jinpu
quoted hunk
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/virtio_blk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
        struct virtio_blk_outhdr out_hdr;
        struct virtio_scsi_inhdr in_hdr;
        u8 status;
+       u8 sense[SCSI_SENSE_BUFFERSIZE];
        struct scatterlist sg[];
 };

@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
        }

        if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-               sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+               memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+               sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
                sgs[num_out + num_in++] = &sense;
                sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
                sgs[num_out + num_in++] = &inhdr;
--
2.1.4

Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: Christoph Hellwig <hch@lst.de>
Date: 2017-01-09 13:35:58 

Is someone going to pick the patch up and send it to Linus?  I keep
running into all kinds of boot failures whenever I forget to cherry
pick it into my development trees..

Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: Jens Axboe <axboe@kernel.dk>
Date: 2017-01-09 15:57:10 

On 01/09/2017 06:35 AM, Christoph Hellwig wrote:
Is someone going to pick the patch up and send it to Linus?  I keep
running into all kinds of boot failures whenever I forget to cherry
pick it into my development trees..
I'll add it.

-- 
Jens Axboe

Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2017-01-09 16:33:18 

On Wed, Jan 04, 2017 at 08:25:05AM +0300, Christoph Hellwig wrote:
quoted hunk
Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/virtio_blk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
 	struct virtio_blk_outhdr out_hdr;
 	struct virtio_scsi_inhdr in_hdr;
 	u8 status;
+	u8 sense[SCSI_SENSE_BUFFERSIZE];
 	struct scatterlist sg[];
 };
 
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
 	}
 
 	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
I would prefer sizeof(vbr->sense) here to avoid duplication.
Otherwise:

Acked-by: Michael S. Tsirkin <mst@redhat.com>


 		sgs[num_out + num_in++] = &sense;
 		sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
 		sgs[num_out + num_in++] = &inhdr;
-- 
2.1.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help