* [PATCH] SG_SET_RESERVED_SIZE negative oops
@ 2003-10-20 22:29 Pat LaVarre
2003-10-20 22:32 ` Pat LaVarre
2003-10-20 22:42 ` Douglas Gilbert
0 siblings, 2 replies; 3+ messages in thread
From: Pat LaVarre @ 2003-10-20 22:29 UTC (permalink / raw)
To: dougg; +Cc: linux-scsi
Doug G:
I propose the 2.6.0-test8 two-line patch below to teach
SG_SET_RESERVED_SIZE to reject a negative size, rather than oops-ing.
Whaddyathink?
I dreamed up this trivial patch after losing my console whenever I typed
something like:
sg_dd of=/dev/sg0 bs=2k bpt=
Courtesy some Red Hat automagic that does a `sudo chown `id -u`
/dev/sg0` to me, I find I can oops that way without involving full root
privilege.
My short nightmare appears detailed as the linux-scsi thread re "sg_dd
bpt= count=":
http://marc.theaimsgroup.com/?t=106617534400002
That thread tells us `sg_dd of=/dev/sg0 bs=2k bpt=-1` is a less
indeterminate way to cause such trouble. Also a two-paragraph demo .c
app appears there to demo trouble even when you have an sg_dd patched to
stop passing thru negative lengths.
In place of the oops, this 2.6.0-test8 patch gives me:
$
$ sg_dd of=/dev/sg0 bs=2k bpt=
unrecognized multiplier
sg_dd: SG_SET_RESERVED_SIZE error: Invalid argument
Not enough user memory
$
Pat LaVarre
diff -Nur linux-2.6.0-test8/drivers/scsi/sg.c linux/drivers/scsi/sg.c
--- linux-2.6.0-test8/drivers/scsi/sg.c 2003-10-17 15:43:10.000000000 -0600
+++ linux/drivers/scsi/sg.c 2003-10-20 16:15:17.699475136 -0600
@@ -877,6 +877,8 @@
result = get_user(val, (int *) arg);
if (result)
return result;
+ if (val < 0)
+ return -EINVAL;
if (val != sfp->reserve.bufflen) {
if (sg_res_in_use(sfp) || sfp->mmap_called)
return -EBUSY;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] SG_SET_RESERVED_SIZE negative oops
2003-10-20 22:29 [PATCH] SG_SET_RESERVED_SIZE negative oops Pat LaVarre
@ 2003-10-20 22:32 ` Pat LaVarre
2003-10-20 22:42 ` Douglas Gilbert
1 sibling, 0 replies; 3+ messages in thread
From: Pat LaVarre @ 2003-10-20 22:32 UTC (permalink / raw)
To: dougg; +Cc: linux-scsi
[ BC axboe@suse.de ]
Here now also is the two-line drivers/block/scsi_ioctl.c patch
corresponding to the drivers/scsi/sg.c patch I proposed to begin this
thread.
I'm guessing we do Not want to bother with scsi_ioctl.c ... right? I
guess this because:
http://lxr.linux.no/search?v=2.6.0-test7&string=sg_reserved_size
persuades me that SG_SET_RESERVED_SIZE does little more than pass an int
over to SG_GET_RESERVED_SIZE ... true?
We have no need to range check that. Therefore we should not bother?
Or are we instead indeed trying to keep drivers/scsi/sg.c in sync with
drivers/block/scsi_ioctl.c?
Pat LaVarre
diff -Nur linux-2.6.0-test8/drivers/block/scsi_ioctl.c linux/drivers/block/scsi_ioctl.c
--- linux-2.6.0-test8/drivers/block/scsi_ioctl.c 2003-10-17 15:43:36.000000000 -0600
+++ linux/drivers/block/scsi_ioctl.c 2003-10-20 16:15:03.898573192 -0600
@@ -124,6 +124,8 @@
if (err)
return err;
+ if (size < 0)
+ return -EINVAL;
if (size > (q->max_sectors << 9))
return -EINVAL;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] SG_SET_RESERVED_SIZE negative oops
2003-10-20 22:29 [PATCH] SG_SET_RESERVED_SIZE negative oops Pat LaVarre
2003-10-20 22:32 ` Pat LaVarre
@ 2003-10-20 22:42 ` Douglas Gilbert
1 sibling, 0 replies; 3+ messages in thread
From: Douglas Gilbert @ 2003-10-20 22:42 UTC (permalink / raw)
To: Pat LaVarre; +Cc: linux-scsi
Pat LaVarre wrote:
> Doug G:
>
> I propose the 2.6.0-test8 two-line patch below to teach
> SG_SET_RESERVED_SIZE to reject a negative size, rather than oops-ing.
>
> Whaddyathink?
>
> I dreamed up this trivial patch after losing my console whenever I typed
> something like:
>
> sg_dd of=/dev/sg0 bs=2k bpt=
>
> Courtesy some Red Hat automagic that does a `sudo chown `id -u`
> /dev/sg0` to me, I find I can oops that way without involving full root
> privilege.
>
> My short nightmare appears detailed as the linux-scsi thread re "sg_dd
> bpt= count=":
> http://marc.theaimsgroup.com/?t=106617534400002
>
> That thread tells us `sg_dd of=/dev/sg0 bs=2k bpt=-1` is a less
> indeterminate way to cause such trouble. Also a two-paragraph demo .c
> app appears there to demo trouble even when you have an sg_dd patched to
> stop passing thru negative lengths.
>
> In place of the oops, this 2.6.0-test8 patch gives me:
>
> $
> $ sg_dd of=/dev/sg0 bs=2k bpt=
> unrecognized multiplier
> sg_dd: SG_SET_RESERVED_SIZE error: Invalid argument
> Not enough user memory
> $
>
> Pat LaVarre
>
> diff -Nur linux-2.6.0-test8/drivers/scsi/sg.c linux/drivers/scsi/sg.c
> --- linux-2.6.0-test8/drivers/scsi/sg.c 2003-10-17 15:43:10.000000000 -0600
> +++ linux/drivers/scsi/sg.c 2003-10-20 16:15:17.699475136 -0600
> @@ -877,6 +877,8 @@
> result = get_user(val, (int *) arg);
> if (result)
> return result;
> + if (val < 0)
> + return -EINVAL;
> if (val != sfp->reserve.bufflen) {
> if (sg_res_in_use(sfp) || sfp->mmap_called)
> return -EBUSY;
>
Pat,
Looks fine to me. I'll send a patch for lk 2.4 later.
Doug Gilbert
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2003-10-20 22:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-10-20 22:29 [PATCH] SG_SET_RESERVED_SIZE negative oops Pat LaVarre
2003-10-20 22:32 ` Pat LaVarre
2003-10-20 22:42 ` Douglas Gilbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).