| Message ID | 20171025095617.7315-1-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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.
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).
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.
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.
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, };
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(+)