Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On 05/23/2017 04:39 PM, Jens Axboe wrote:
> I tried to look up that commit:
>
> 48ae8484e9fc ("scsi: sg: don't return bogus Sg_requests")
>
> but that isn't in Linus' tree. Even searched for just the title, still
> didn't find anything.
It's queued up in Martin's tree [1].
>
> I'm assuming this is a bug in the sg.c driver, in which case the 2/3
> prep and real test case looks fine. For generic device testing, we
> should just use SG_IO and not bother with sg.c at all. The world would
> be a better place if we could just get rid of sg.c...
Agreed. Yes the bug is in the sg.c driver and we did have quite some of
these lately thanks to the syzcaller folks.
My intention with these tests was to have a place where we can throw in
the syzcaller reproducers and run it in nicely Qemu.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=4.12/scsi-fixes=48ae8484e9fc324b4968d33c585e54bc98e44d61
--
Johannes Thumshirn Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On 05/23/2017 08:25 AM, Johannes Thumshirn wrote:
> On 05/23/2017 04:15 PM, Jens Axboe wrote:
>> Add some code to the framework that allows you to get the corresponding
>> SG device for a SCSI block device? Make that part of the prepare, skip
>> the test if the block device isn't a SCSI dev.
>
> Well the code is already there (in patch 2/3).
>
> I'll pack it into the prepare stage.
I tried to look up that commit:
48ae8484e9fc ("scsi: sg: don't return bogus Sg_requests")
but that isn't in Linus' tree. Even searched for just the title, still
didn't find anything.
I'm assuming this is a bug in the sg.c driver, in which case the 2/3
prep and real test case looks fine. For generic device testing, we
should just use SG_IO and not bother with sg.c at all. The world would
be a better place if we could just get rid of sg.c...
--
Jens Axboe
Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On 05/23/2017 04:15 PM, Jens Axboe wrote: > Add some code to the framework that allows you to get the corresponding > SG device for a SCSI block device? Make that part of the prepare, skip > the test if the block device isn't a SCSI dev. Well the code is already there (in patch 2/3). I'll pack it into the prepare stage. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On 05/23/2017 12:58 AM, Johannes Thumshirn wrote: > On 05/22/2017 07:59 PM, Omar Sandoval wrote: >> This looks much better, thanks! One question for you: is there any value >> in running this on specific test devices (i.e., changing test() to >> test_device() and using "$TEST_DEV" instead of a scsi-debug device), or >> would it be a waste of time since it's just exercising generic code? > > That's just generic code. All I need is a SCSI device so I get a /dev/sg > device node. > > One could do a check if $TEST_DEV is a SCSI device and have a fall-back > to scsi_debug if it isn't, but I'm not sure if this isn't just a waste > of time. Add some code to the framework that allows you to get the corresponding SG device for a SCSI block device? Make that part of the prepare, skip the test if the block device isn't a SCSI dev. -- Jens Axboe
Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On 05/22/2017 07:59 PM, Omar Sandoval wrote: > This looks much better, thanks! One question for you: is there any value > in running this on specific test devices (i.e., changing test() to > test_device() and using "$TEST_DEV" instead of a scsi-debug device), or > would it be a waste of time since it's just exercising generic code? That's just generic code. All I need is a SCSI device so I get a /dev/sg device node. One could do a check if $TEST_DEV is a SCSI device and have a fall-back to scsi_debug if it isn't, but I'm not sure if this isn't just a waste of time. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path
On Fri, May 19, 2017 at 03:55:31PM +0200, Johannes Thumshirn wrote:
> Add a regression test for commit 48ae8484e9fc ("scsi: sg: don't return
> bogus Sg_requests"). This is a general protection fault triggered by
> syzcaller via issuing bogus read(2)s on the /dev/sg devices.
>
> Signed-off-by: Johannes Thumshirn
> ---
> tests/sg/001 | 47 +++
> tests/sg/001.out | 2 ++
> 2 files changed, 49 insertions(+)
> create mode 100755 tests/sg/001
> create mode 100644 tests/sg/001.out
>
> diff --git a/tests/sg/001 b/tests/sg/001
> new file mode 100755
> index ..86430409b6a3
> --- /dev/null
> +++ b/tests/sg/001
> @@ -0,0 +1,47 @@
> +#!/bin/bash
> +#
> +# Regression test for commit 48ae8484e9fc ("scsi: sg: don't return bogus
> +# Sg_requests")
> +#
> +# Copyright (C) 2017 Johannes Thumshirn
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see
