All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] virtio_blk: Fix an SG_IO regression
@ 2017-10-25  9:56 Bart Van Assche
  2017-10-25 18:23 ` Jens Axboe
  0 siblings, 1 reply; 11+ messages in thread
From: Bart Van Assche @ 2017-10-25  9:56 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche,
	Michael S . Tsirkin, Dann Frazier, stable

Avoid that submitting an SG_IO ioctl triggers a kernel oops that
is preceded by:

usercopy: kernel memory overwrite attempt detected to (null) (<null>) (6 bytes)
kernel BUG at mm/usercopy.c:72!

Reported-by: Dann Frazier <dann.frazier@canonical.com>
Fixes: commit ca18d6f769d2 ("block: Make most scsi_req_init() calls implicit")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Tested-by: Dann Frazier <dann.frazier@canonical.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Dann Frazier <dann.frazier@canonical.com>
Cc: <stable@vger.kernel.org> # v4.13
---
 drivers/block/Kconfig      | 1 +
 drivers/block/virtio_blk.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 4a438b8abe27..b0b2100763bf 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -450,6 +450,7 @@ config VIRTIO_BLK_SCSI
 	bool "SCSI passthrough request for the Virtio block driver"
 	depends on VIRTIO_BLK
 	select BLK_SCSI_REQUEST
+	select SCSI_MOD
 	---help---
 	  Enable support for SCSI passthrough (e.g. the SG_IO ioctl) on
 	  virtio-blk devices.  This is only supported for the legacy
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 34e17ee799be..15e11a519801 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -597,6 +597,7 @@ static const struct blk_mq_ops virtio_mq_ops = {
 	.queue_rq	= virtio_queue_rq,
 	.complete	= virtblk_request_done,
 	.init_request	= virtblk_init_request,
+	.initialize_rq_fn = scsi_initialize_rq,
 	.map_queues	= virtblk_map_queues,
 };
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25  9:56 [PATCH v2] virtio_blk: Fix an SG_IO regression Bart Van Assche
@ 2017-10-25 18:23 ` Jens Axboe
  2017-10-25 19:25     ` Bart Van Assche
  0 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2017-10-25 18:23 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-block, Christoph Hellwig, Michael S . Tsirkin, Dann Frazier,
	stable

On 10/25/2017 02:56 AM, Bart Van Assche wrote:
> Avoid that submitting an SG_IO ioctl triggers a kernel oops that
> is preceded by:
> 
> usercopy: kernel memory overwrite attempt detected to (null) (<null>) (6 bytes)
> kernel BUG at mm/usercopy.c:72!

Seems I saw a note on a runtime oops triggered by this patch yesterday,
but now I can't seem to find it... Did you see it?

> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 4a438b8abe27..b0b2100763bf 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -450,6 +450,7 @@ config VIRTIO_BLK_SCSI
>  	bool "SCSI passthrough request for the Virtio block driver"
>  	depends on VIRTIO_BLK
>  	select BLK_SCSI_REQUEST
> +	select SCSI_MOD

Should this be SCSI? That's what libata does. It may be correct as-is,
didn't look too deeply, just curious why it's different.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25 18:23 ` Jens Axboe
@ 2017-10-25 19:25     ` Bart Van Assche
  0 siblings, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-25 19:25 UTC (permalink / raw)
  To: axboe@kernel.dk
  Cc: hch@lst.de, linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

T24gV2VkLCAyMDE3LTEwLTI1IGF0IDExOjIzIC0wNzAwLCBKZW5zIEF4Ym9lIHdyb3RlOg0KPiBP
biAxMC8yNS8yMDE3IDAyOjU2IEFNLCBCYXJ0IFZhbiBBc3NjaGUgd3JvdGU6DQo+ID4gQXZvaWQg
dGhhdCBzdWJtaXR0aW5nIGFuIFNHX0lPIGlvY3RsIHRyaWdnZXJzIGEga2VybmVsIG9vcHMgdGhh
dA0KPiA+IGlzIHByZWNlZGVkIGJ5Og0KPiA+IA0KPiA+IHVzZXJjb3B5OiBrZXJuZWwgbWVtb3J5
IG92ZXJ3cml0ZSBhdHRlbXB0IGRldGVjdGVkIHRvIChudWxsKSAoPG51bGw+KSAoNiBieXRlcykN
Cj4gPiBrZXJuZWwgQlVHIGF0IG1tL3VzZXJjb3B5LmM6NzIhDQo+IA0KPiBTZWVtcyBJIHNhdyBh
IG5vdGUgb24gYSBydW50aW1lIG9vcHMgdHJpZ2dlcmVkIGJ5IHRoaXMgcGF0Y2ggeWVzdGVyZGF5
LA0KPiBidXQgbm93IEkgY2FuJ3Qgc2VlbSB0byBmaW5kIGl0Li4uIERpZCB5b3Ugc2VlIGl0Pw0K
DQpEbyB5b3UgcGVyaGFwcyB3YW50IG1lIHRvIGFkZCB0aGUgc3RhY2sgdHJhY2UgZnJvbSB0aGUg
Zm9sbG93aW5nIGUtbWFpbCB0bw0KdGhlIHBhdGNoIGRlc2NyaXB0aW9uOiBodHRwczovL21hcmMu
aW5mby8/bD1saW51eC1hcm0ta2VybmVsJm09MTUwODU0MDEwMzIxODMzID8NCg0KPiA+IGRpZmYg
LS1naXQgYS9kcml2ZXJzL2Jsb2NrL0tjb25maWcgYi9kcml2ZXJzL2Jsb2NrL0tjb25maWcNCj4g
PiBpbmRleCA0YTQzOGI4YWJlMjcuLmIwYjIxMDA3NjNiZiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2
ZXJzL2Jsb2NrL0tjb25maWcNCj4gPiArKysgYi9kcml2ZXJzL2Jsb2NrL0tjb25maWcNCj4gPiBA
QCAtNDUwLDYgKzQ1MCw3IEBAIGNvbmZpZyBWSVJUSU9fQkxLX1NDU0kNCj4gPiAgCWJvb2wgIlND
U0kgcGFzc3Rocm91Z2ggcmVxdWVzdCBmb3IgdGhlIFZpcnRpbyBibG9jayBkcml2ZXIiDQo+ID4g
IAlkZXBlbmRzIG9uIFZJUlRJT19CTEsNCj4gPiAgCXNlbGVjdCBCTEtfU0NTSV9SRVFVRVNUDQo+
ID4gKwlzZWxlY3QgU0NTSV9NT0QNCj4gDQo+IFNob3VsZCB0aGlzIGJlIFNDU0k/IFRoYXQncyB3
aGF0IGxpYmF0YSBkb2VzLiBJdCBtYXkgYmUgY29ycmVjdCBhcy1pcywNCj4gZGlkbid0IGxvb2sg
dG9vIGRlZXBseSwganVzdCBjdXJpb3VzIHdoeSBpdCdzIGRpZmZlcmVudC4NCg0KVGhhdCBpcyB3
aGF0IEkgY2FtZSB1cCB3aXRoIGFmdGVyIGhhdmluZyBoYWQgYSBsb29rIGF0IGRyaXZlcnMvc2Nz
aS9NYWtlZmlsZS4NCkJ1dCBhZnRlciBoYXZpbmcgY2hlY2tlZCBkcml2ZXJzL3Njc2kvS2NvbmZp
ZyBJIHRoaW5rIHdlIGluZGVlZCBuZWVkIHRvIHNlbGVjdA0KU0NTSSBpbnN0ZWFkIG9mIFNDU0lf
TU9ELg0KDQpCYXJ0Lg==

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
@ 2017-10-25 19:25     ` Bart Van Assche
  0 siblings, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-25 19:25 UTC (permalink / raw)
  To: axboe@kernel.dk
  Cc: hch@lst.de, linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

On Wed, 2017-10-25 at 11:23 -0700, Jens Axboe wrote:
> On 10/25/2017 02:56 AM, Bart Van Assche wrote:
> > Avoid that submitting an SG_IO ioctl triggers a kernel oops that
> > is preceded by:
> > 
> > usercopy: kernel memory overwrite attempt detected to (null) (<null>) (6 bytes)
> > kernel BUG at mm/usercopy.c:72!
> 
> Seems I saw a note on a runtime oops triggered by this patch yesterday,
> but now I can't seem to find it... Did you see it?

Do you perhaps want me to add the stack trace from the following e-mail to
the patch description: https://marc.info/?l=linux-arm-kernel&m=150854010321833 ?

> > diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> > index 4a438b8abe27..b0b2100763bf 100644
> > --- a/drivers/block/Kconfig
> > +++ b/drivers/block/Kconfig
> > @@ -450,6 +450,7 @@ config VIRTIO_BLK_SCSI
> >  	bool "SCSI passthrough request for the Virtio block driver"
> >  	depends on VIRTIO_BLK
> >  	select BLK_SCSI_REQUEST
> > +	select SCSI_MOD
> 
> Should this be SCSI? That's what libata does. It may be correct as-is,
> didn't look too deeply, just curious why it's different.

That is what I came up with after having had a look at drivers/scsi/Makefile.
But after having checked drivers/scsi/Kconfig I think we indeed need to select
SCSI instead of SCSI_MOD.

Bart.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25 19:25     ` Bart Van Assche
  (?)
@ 2017-10-25 19:34     ` Jens Axboe
  2017-10-25 19:37       ` hch
  2017-10-26  4:44         ` Bart Van Assche
  -1 siblings, 2 replies; 11+ messages in thread
From: Jens Axboe @ 2017-10-25 19:34 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: hch@lst.de, linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

On 10/25/2017 12:25 PM, Bart Van Assche wrote:
> On Wed, 2017-10-25 at 11:23 -0700, Jens Axboe wrote:
>> On 10/25/2017 02:56 AM, Bart Van Assche wrote:
>>> Avoid that submitting an SG_IO ioctl triggers a kernel oops that
>>> is preceded by:
>>>
>>> usercopy: kernel memory overwrite attempt detected to (null) (<null>) (6 bytes)
>>> kernel BUG at mm/usercopy.c:72!
>>
>> Seems I saw a note on a runtime oops triggered by this patch yesterday,
>> but now I can't seem to find it... Did you see it?
> 
> Do you perhaps want me to add the stack trace from the following e-mail to
> the patch description: https://marc.info/?l=linux-arm-kernel&m=150854010321833 ?

It was an oops reported against the current patch, unless I'm mistaken. Hard
to say, when I can't find the email this morning, may have been deleted...
That's why I asked if you saw it.

>>> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
>>> index 4a438b8abe27..b0b2100763bf 100644
>>> --- a/drivers/block/Kconfig
>>> +++ b/drivers/block/Kconfig
>>> @@ -450,6 +450,7 @@ config VIRTIO_BLK_SCSI
>>>  	bool "SCSI passthrough request for the Virtio block driver"
>>>  	depends on VIRTIO_BLK
>>>  	select BLK_SCSI_REQUEST
>>> +	select SCSI_MOD
>>
>> Should this be SCSI? That's what libata does. It may be correct as-is,
>> didn't look too deeply, just curious why it's different.
> 
> That is what I came up with after having had a look at drivers/scsi/Makefile.
> But after having checked drivers/scsi/Kconfig I think we indeed need to select
> SCSI instead of SCSI_MOD.

I think so.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25 19:34     ` Jens Axboe
@ 2017-10-25 19:37       ` hch
  2017-10-26  4:33           ` Bart Van Assche
  2017-10-26  4:44         ` Bart Van Assche
  1 sibling, 1 reply; 11+ messages in thread
From: hch @ 2017-10-25 19:37 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Bart Van Assche, hch@lst.de, linux-block@vger.kernel.org,
	mst@redhat.com, stable@vger.kernel.org,
	dann.frazier@canonical.com

Honestly I think the right fix is to just kill the SCSI passthrough
in virtio.  It has been replaced by virtio-scsi a long time ago, and
has been disabled by default in qemu for a long time (and I don't think
any other hypervisor ever supported it).

It should never have been added (saying that as the person who added it).

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25 19:37       ` hch
@ 2017-10-26  4:33           ` Bart Van Assche
  0 siblings, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-26  4:33 UTC (permalink / raw)
  To: hch@lst.de, axboe@kernel.dk
  Cc: linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

T24gV2VkLCAyMDE3LTEwLTI1IGF0IDIxOjM3ICswMjAwLCBoY2hAbHN0LmRlIHdyb3RlOg0KPiBI
b25lc3RseSBJIHRoaW5rIHRoZSByaWdodCBmaXggaXMgdG8ganVzdCBraWxsIHRoZSBTQ1NJIHBh
c3N0aHJvdWdoDQo+IGluIHZpcnRpby4gIEl0IGhhcyBiZWVuIHJlcGxhY2VkIGJ5IHZpcnRpby1z
Y3NpIGEgbG9uZyB0aW1lIGFnbywgYW5kDQo+IGhhcyBiZWVuIGRpc2FibGVkIGJ5IGRlZmF1bHQg
aW4gcWVtdSBmb3IgYSBsb25nIHRpbWUgKGFuZCBJIGRvbid0DQo+IHRoaW5rIGFueSBvdGhlciBo
eXBlcnZpc29yIGV2ZXIgc3VwcG9ydGVkIGl0KS4NCj4gDQo+IEl0IHNob3VsZCBuZXZlciBoYXZl
IGJlZW4gYWRkZWQgKHNheWluZyB0aGF0IGFzIHRoZSBwZXJzb24gd2hvIGFkZGVkDQo+IGl0KS4N
Cg0KSGVsbG8gQ2hyaXN0b3BoLA0KDQpTb3JyeSBidXQgSSdtIG5vdCBmYW1pbGlhciBlbm91Z2gg
d2l0aCB0aGUgdmlydGlvX2JsayBkcml2ZXIgdG8gZG8gdGhhdA0KcmVtb3ZhbCBteXNlbGYuIFNp
bmNlIHRoZSBwYXRjaCBhdCB0aGUgc3RhcnQgb2YgdGhpcyBlLW1haWwgdGhyZWFkIGhhcw0KYSAi
Q2M6IHN0YWJsZSIgdGFnLCBjYW4geW91IHN1Ym1pdCBzdWNoIGEgcGF0Y2ggYWZ0ZXIgdGhpcyBw
YXRjaCB3ZW50DQppbj8NCg0KVGhhbmtzLA0KDQpCYXJ0Lg==

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
@ 2017-10-26  4:33           ` Bart Van Assche
  0 siblings, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-26  4:33 UTC (permalink / raw)
  To: hch@lst.de, axboe@kernel.dk
  Cc: linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

On Wed, 2017-10-25 at 21:37 +0200, hch@lst.de wrote:
> Honestly I think the right fix is to just kill the SCSI passthrough
> in virtio.  It has been replaced by virtio-scsi a long time ago, and
> has been disabled by default in qemu for a long time (and I don't
> think any other hypervisor ever supported it).
> 
> It should never have been added (saying that as the person who added
> it).

Hello Christoph,

Sorry but I'm not familiar enough with the virtio_blk driver to do that
removal myself. Since the patch at the start of this e-mail thread has
a "Cc: stable" tag, can you submit such a patch after this patch went
in?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-26  4:33           ` Bart Van Assche
  (?)
@ 2017-10-26  4:38           ` Jens Axboe
  -1 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-10-26  4:38 UTC (permalink / raw)
  To: Bart Van Assche, hch@lst.de
  Cc: linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

On 10/25/2017 09:33 PM, Bart Van Assche wrote:
> On Wed, 2017-10-25 at 21:37 +0200, hch@lst.de wrote:
>> Honestly I think the right fix is to just kill the SCSI passthrough
>> in virtio.  It has been replaced by virtio-scsi a long time ago, and
>> has been disabled by default in qemu for a long time (and I don't
>> think any other hypervisor ever supported it).
>>
>> It should never have been added (saying that as the person who added
>> it).
> 
> Hello Christoph,
> 
> Sorry but I'm not familiar enough with the virtio_blk driver to do that
> removal myself. Since the patch at the start of this e-mail thread has
> a "Cc: stable" tag, can you submit such a patch after this patch went
> in?

Something like the below. But yes, we should probably get the simple fix
in, then kill it after, since the fix is targeting stable and the
current series.

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 2dfe99b328f8..9fac2b724577 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -446,16 +446,6 @@ config VIRTIO_BLK
 	  This is the virtual block driver for virtio.  It can be used with
           QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
-config VIRTIO_BLK_SCSI
-	bool "SCSI passthrough request for the Virtio block driver"
-	depends on VIRTIO_BLK
-	select BLK_SCSI_REQUEST
-	---help---
-	  Enable support for SCSI passthrough (e.g. the SG_IO ioctl) on
-	  virtio-blk devices.  This is only supported for the legacy
-	  virtio protocol and not enabled by default by any hypervisor.
-	  You probably want to use virtio-scsi instead.
-
 config BLK_DEV_RBD
 	tristate "Rados block device (RBD)"
 	depends on INET && BLOCK
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 34e17ee799be..1764df4f1c60 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -10,7 +10,6 @@
 #include <linux/virtio_blk.h>
 #include <linux/scatterlist.h>
 #include <linux/string_helpers.h>
-#include <scsi/scsi_cmnd.h>
 #include <linux/idr.h>
 #include <linux/blk-mq.h>
 #include <linux/blk-mq-virtio.h>
@@ -54,11 +53,6 @@ struct virtio_blk {
 };
 
 struct virtblk_req {
-#ifdef CONFIG_VIRTIO_BLK_SCSI
-	struct scsi_request sreq;	/* for SCSI passthrough, must be first */
-	u8 sense[SCSI_SENSE_BUFFERSIZE];
-	struct virtio_scsi_inhdr in_hdr;
-#endif
 	struct virtio_blk_outhdr out_hdr;
 	u8 status;
 	struct scatterlist sg[];
@@ -76,80 +70,6 @@ static inline blk_status_t virtblk_result(struct virtblk_req *vbr)
 	}
 }
 
-/*
- * If this is a packet command we need a couple of additional headers.  Behind
- * the normal outhdr we put a segment with the scsi command block, and before
- * the normal inhdr we put the sense data and the inhdr with additional status
- * information.
- */
-#ifdef CONFIG_VIRTIO_BLK_SCSI
-static int virtblk_add_req_scsi(struct virtqueue *vq, struct virtblk_req *vbr,
-		struct scatterlist *data_sg, bool have_data)
-{
-	struct scatterlist hdr, status, cmd, sense, inhdr, *sgs[6];
-	unsigned int num_out = 0, num_in = 0;
-
-	sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
-	sgs[num_out++] = &hdr;
-	sg_init_one(&cmd, vbr->sreq.cmd, vbr->sreq.cmd_len);
-	sgs[num_out++] = &cmd;
-
-	if (have_data) {
-		if (vbr->out_hdr.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT))
-			sgs[num_out++] = data_sg;
-		else
-			sgs[num_out + num_in++] = data_sg;
-	}
-
-	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;
-	sg_init_one(&status, &vbr->status, sizeof(vbr->status));
-	sgs[num_out + num_in++] = &status;
-
-	return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
-}
-
-static inline void virtblk_scsi_request_done(struct request *req)
-{
-	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
-	struct virtio_blk *vblk = req->q->queuedata;
-	struct scsi_request *sreq = &vbr->sreq;
-
-	sreq->resid_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual);
-	sreq->sense_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
-	sreq->result = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors);
-}
-
-static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
-			     unsigned int cmd, unsigned long data)
-{
-	struct gendisk *disk = bdev->bd_disk;
-	struct virtio_blk *vblk = disk->private_data;
-
-	/*
-	 * Only allow the generic SCSI ioctls if the host can support it.
-	 */
-	if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_SCSI))
-		return -ENOTTY;
-
-	return scsi_cmd_blk_ioctl(bdev, mode, cmd,
-				  (void __user *)data);
-}
-#else
-static inline int virtblk_add_req_scsi(struct virtqueue *vq,
-		struct virtblk_req *vbr, struct scatterlist *data_sg,
-		bool have_data)
-{
-	return -EIO;
-}
-static inline void virtblk_scsi_request_done(struct request *req)
-{
-}
-#define virtblk_ioctl	NULL
-#endif /* CONFIG_VIRTIO_BLK_SCSI */
-
 static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr,
 		struct scatterlist *data_sg, bool have_data)
 {
@@ -176,13 +96,6 @@ static inline void virtblk_request_done(struct request *req)
 {
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
 
-	switch (req_op(req)) {
-	case REQ_OP_SCSI_IN:
-	case REQ_OP_SCSI_OUT:
-		virtblk_scsi_request_done(req);
-		break;
-	}
-
 	blk_mq_end_request(req, virtblk_result(vbr));
 }
 
@@ -237,10 +150,6 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 	case REQ_OP_FLUSH:
 		type = VIRTIO_BLK_T_FLUSH;
 		break;
-	case REQ_OP_SCSI_IN:
-	case REQ_OP_SCSI_OUT:
-		type = VIRTIO_BLK_T_SCSI_CMD;
-		break;
 	case REQ_OP_DRV_IN:
 		type = VIRTIO_BLK_T_GET_ID;
 		break;
@@ -265,10 +174,7 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 	}
 
 	spin_lock_irqsave(&vblk->vqs[qid].lock, flags);
-	if (blk_rq_is_scsi(req))
-		err = virtblk_add_req_scsi(vblk->vqs[qid].vq, vbr, vbr->sg, num);
-	else
-		err = virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num);
+	err = virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num);
 	if (err) {
 		virtqueue_kick(vblk->vqs[qid].vq);
 		blk_mq_stop_hw_queue(hctx);
@@ -336,7 +242,6 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 }
 
 static const struct block_device_operations virtblk_fops = {
-	.ioctl  = virtblk_ioctl,
 	.owner  = THIS_MODULE,
 	.getgeo = virtblk_getgeo,
 };
@@ -579,9 +484,6 @@ static int virtblk_init_request(struct blk_mq_tag_set *set, struct request *rq,
 	struct virtio_blk *vblk = set->driver_data;
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(rq);
 
-#ifdef CONFIG_VIRTIO_BLK_SCSI
-	vbr->sreq.sense = vbr->sense;
-#endif
 	sg_init_table(vbr->sg, vblk->sg_elems);
 	return 0;
 }
@@ -871,9 +773,6 @@ static const struct virtio_device_id id_table[] = {
 static unsigned int features_legacy[] = {
 	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
 	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
-#ifdef CONFIG_VIRTIO_BLK_SCSI
-	VIRTIO_BLK_F_SCSI,
-#endif
 	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
 	VIRTIO_BLK_F_MQ,
 }

-- 
Jens Axboe

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
  2017-10-25 19:34     ` Jens Axboe
@ 2017-10-26  4:44         ` Bart Van Assche
  2017-10-26  4:44         ` Bart Van Assche
  1 sibling, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-26  4:44 UTC (permalink / raw)
  To: axboe@kernel.dk
  Cc: hch@lst.de, linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

T24gV2VkLCAyMDE3LTEwLTI1IGF0IDEyOjM0IC0wNzAwLCBKZW5zIEF4Ym9lIHdyb3RlOg0KPiBP
biAxMC8yNS8yMDE3IDEyOjI1IFBNLCBCYXJ0IFZhbiBBc3NjaGUgd3JvdGU6DQo+ID4gT24gV2Vk
LCAyMDE3LTEwLTI1IGF0IDExOjIzIC0wNzAwLCBKZW5zIEF4Ym9lIHdyb3RlOg0KPiA+ID4gT24g
MTAvMjUvMjAxNyAwMjo1NiBBTSwgQmFydCBWYW4gQXNzY2hlIHdyb3RlOg0KPiA+ID4gPiBBdm9p
ZCB0aGF0IHN1Ym1pdHRpbmcgYW4gU0dfSU8gaW9jdGwgdHJpZ2dlcnMgYSBrZXJuZWwgb29wcyB0
aGF0DQo+ID4gPiA+IGlzIHByZWNlZGVkIGJ5Og0KPiA+ID4gPiANCj4gPiA+ID4gdXNlcmNvcHk6
IGtlcm5lbCBtZW1vcnkgb3ZlcndyaXRlIGF0dGVtcHQgZGV0ZWN0ZWQgdG8gKG51bGwpICg8bnVs
bD4pICg2IGJ5dGVzKQ0KPiA+ID4gPiBrZXJuZWwgQlVHIGF0IG1tL3VzZXJjb3B5LmM6NzIhDQo+
ID4gPiANCj4gPiA+IFNlZW1zIEkgc2F3IGEgbm90ZSBvbiBhIHJ1bnRpbWUgb29wcyB0cmlnZ2Vy
ZWQgYnkgdGhpcyBwYXRjaCB5ZXN0ZXJkYXksDQo+ID4gPiBidXQgbm93IEkgY2FuJ3Qgc2VlbSB0
byBmaW5kIGl0Li4uIERpZCB5b3Ugc2VlIGl0Pw0KPiA+IA0KPiA+IERvIHlvdSBwZXJoYXBzIHdh
bnQgbWUgdG8gYWRkIHRoZSBzdGFjayB0cmFjZSBmcm9tIHRoZSBmb2xsb3dpbmcgZS1tYWlsIHRv
DQo+ID4gdGhlIHBhdGNoIGRlc2NyaXB0aW9uOiBodHRwczovL21hcmMuaW5mby8/bD1saW51eC1h
cm0ta2VybmVsJm09MTUwODU0MDEwMzIxODMzID8NCj4gDQo+IEl0IHdhcyBhbiBvb3BzIHJlcG9y
dGVkIGFnYWluc3QgdGhlIGN1cnJlbnQgcGF0Y2gsIHVubGVzcyBJJ20gbWlzdGFrZW4uIEhhcmQN
Cj4gdG8gc2F5LCB3aGVuIEkgY2FuJ3QgZmluZCB0aGUgZW1haWwgdGhpcyBtb3JuaW5nLCBtYXkg
aGF2ZSBiZWVuIGRlbGV0ZWQuLi4NCj4gVGhhdCdzIHdoeSBJIGFza2VkIGlmIHlvdSBzYXcgaXQu
DQoNCkp1c3QgZm91bmQgaXQ6IGE1NzA4NDNlZTkgKCJ2aXJ0aW9fYmxrOiBGaXggYW4gU0dfSU8g
cmVncmVzc2lvbiIpOiBrZXJuZWwgQlVHIGF0DQppbmNsdWRlL2xpbnV4L3NjYXR0ZXJsaXN0Lmg6
OTIhIChodHRwczovL2xrbWwub3JnL2xrbWwvMjAxNy8xMC8yNC8xMTE3KS4gV2lsbCBoYXZlDQph
IGxvb2suDQoNCkJhcnQuDQo=

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] virtio_blk: Fix an SG_IO regression
@ 2017-10-26  4:44         ` Bart Van Assche
  0 siblings, 0 replies; 11+ messages in thread
From: Bart Van Assche @ 2017-10-26  4:44 UTC (permalink / raw)
  To: axboe@kernel.dk
  Cc: hch@lst.de, linux-block@vger.kernel.org, mst@redhat.com,
	stable@vger.kernel.org, dann.frazier@canonical.com

On Wed, 2017-10-25 at 12:34 -0700, Jens Axboe wrote:
> On 10/25/2017 12:25 PM, Bart Van Assche wrote:
> > On Wed, 2017-10-25 at 11:23 -0700, Jens Axboe wrote:
> > > On 10/25/2017 02:56 AM, Bart Van Assche wrote:
> > > > Avoid that submitting an SG_IO ioctl triggers a kernel oops that
> > > > is preceded by:
> > > > 
> > > > usercopy: kernel memory overwrite attempt detected to (null) (<null>) (6 bytes)
> > > > kernel BUG at mm/usercopy.c:72!
> > > 
> > > Seems I saw a note on a runtime oops triggered by this patch yesterday,
> > > but now I can't seem to find it... Did you see it?
> > 
> > Do you perhaps want me to add the stack trace from the following e-mail to
> > the patch description: https://marc.info/?l=linux-arm-kernel&m=150854010321833 ?
> 
> It was an oops reported against the current patch, unless I'm mistaken. Hard
> to say, when I can't find the email this morning, may have been deleted...
> That's why I asked if you saw it.

Just found it: a570843ee9 ("virtio_blk: Fix an SG_IO regression"): kernel BUG at
include/linux/scatterlist.h:92! (https://lkml.org/lkml/2017/10/24/1117). Will have
a look.

Bart.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-26  4:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-25  9:56 [PATCH v2] virtio_blk: Fix an SG_IO regression Bart Van Assche
2017-10-25 18:23 ` Jens Axboe
2017-10-25 19:25   ` Bart Van Assche
2017-10-25 19:25     ` Bart Van Assche
2017-10-25 19:34     ` Jens Axboe
2017-10-25 19:37       ` hch
2017-10-26  4:33         ` Bart Van Assche
2017-10-26  4:33           ` Bart Van Assche
2017-10-26  4:38           ` Jens Axboe
2017-10-26  4:44       ` Bart Van Assche
2017-10-26  4:44         ` Bart Van Assche

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.