* [TRIVIAL] copy_XX_user for sg.c - take 2
@ 2002-07-15 8:19 Rusty Trivial Russell
2002-07-17 3:25 ` Douglas Gilbert
0 siblings, 1 reply; 3+ messages in thread
From: Rusty Trivial Russell @ 2002-07-15 8:19 UTC (permalink / raw)
To: linux-scsi
[ This one is hardly trivial: it seems correct, but... Please check ]
From: (via Rusty) william stinson <wstinson@infonie.fr>
Hi Rusty,
here is version 2 of my sg.c patch to correct indentation to 4blanks as you pointed out (I hope I didn't miss anything).
This proposed patch for the sg.c driver which allows user process control of SCSI devices
1) checks the result of copy_XX_user
2) returns -EFAULT in case not all data was copied
3) releases resources in case of failure
4) return value of sg_read_oxfer changed from void to int in order to be able to pass back error conditions
I don't have this kind of hardware so compilation checked only.
Patch also available at the following URL:
http://www.chez.com/wstinson/linux/kernel/patch-scsi-sg
Have a good weekend!
William Stinson (wstinson@infonie.fr)
--- trivial-2.5.25/drivers/scsi/sg.c.orig Mon Jul 15 17:03:26 2002
+++ trivial-2.5.25/drivers/scsi/sg.c Mon Jul 15 17:03:26 2002
@@ -215,7 +215,7 @@
int wr_xf, int * countp, unsigned char ** up);
static int sg_write_xfer(Sg_request * srp);
static int sg_read_xfer(Sg_request * srp);
-static void sg_read_oxfer(Sg_request * srp, char * outp, int num_read_xfer);
+static int sg_read_oxfer(Sg_request * srp, char * outp, int num_read_xfer);
static void sg_remove_scat(Sg_scatter_hold * schp);
static char * sg_get_sgat_msa(Sg_scatter_hold * schp);
static void sg_build_reserve(Sg_fd * sfp, int req_size);
@@ -377,10 +377,12 @@
if ((k = verify_area(VERIFY_WRITE, buf, count)))
return k;
if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
- __copy_from_user(&old_hdr, buf, SZ_SG_HEADER);
+ if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
+ return -EFAULT;
if (old_hdr.reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- __copy_from_user(&new_hdr, buf, SZ_SG_IO_HDR);
+ if (__copy_from_user(&new_hdr, buf, SZ_SG_IO_HDR))
+ return -EFAULT;
req_pack_id = new_hdr.pack_id;
}
}
@@ -452,12 +454,14 @@
/* Now copy the result back to the user buffer. */
if (count >= SZ_SG_HEADER) {
- __copy_to_user(buf, &old_hdr, SZ_SG_HEADER);
- buf += SZ_SG_HEADER;
+ if (__copy_to_user(buf, &old_hdr, SZ_SG_HEADER))
+ return -EFAULT;
+ buf += SZ_SG_HEADER;
if (count > old_hdr.reply_len)
count = old_hdr.reply_len;
if (count > SZ_SG_HEADER)
- sg_read_oxfer(srp, buf, count - SZ_SG_HEADER);
+ if ((res = sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)))
+ return res;
}
else
count = (old_hdr.result == 0) ? 0 : -EIO;
@@ -486,13 +490,19 @@
len = (len > sb_len) ? sb_len : len;
if ((err = verify_area(VERIFY_WRITE, hp->sbp, len)))
goto err_out;
- __copy_to_user(hp->sbp, srp->sense_b, len);
+ if (__copy_to_user(hp->sbp, srp->sense_b, len)) {
+ err = -EFAULT;
+ goto err_out;
+ }
hp->sb_len_wr = len;
}
}
if (hp->masked_status || hp->host_status || hp->driver_status)
hp->info |= SG_INFO_CHECK;
- copy_to_user(buf, hp, SZ_SG_IO_HDR);
+ if (copy_to_user(buf, hp, SZ_SG_IO_HDR)) {
+ err = -EFAULT;
+ goto err_out;
+ }
err = sg_read_xfer(srp);
err_out:
sg_finish_rem_req(srp);
@@ -529,7 +539,8 @@
return k; /* protects following copy_from_user()s + get_user()s */
if (count < SZ_SG_HEADER)
return -EIO;
- __copy_from_user(&old_hdr, buf, SZ_SG_HEADER);
+ if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
+ return -EFAULT;
blocking = !(filp->f_flags & O_NONBLOCK);
if (old_hdr.reply_len < 0)
return sg_new_write(sfp, buf, count, blocking, 0, NULL);
@@ -587,7 +598,8 @@
hp->flags = input_size; /* structure abuse ... */
hp->pack_id = old_hdr.pack_id;
hp->usr_ptr = NULL;
- __copy_from_user(cmnd, buf, cmd_size);
+ if (__copy_from_user(cmnd, buf, cmd_size))
+ return -EFAULT;
k = sg_common_write(sfp, srp, cmnd, sfp->timeout, blocking);
return (k < 0) ? k : count;
}
@@ -612,7 +624,10 @@
return -EDOM;
}
hp = &srp->header;
- __copy_from_user(hp, buf, SZ_SG_IO_HDR);
+ if (__copy_from_user(hp, buf, SZ_SG_IO_HDR)) {
+ sg_remove_request(sfp, srp);
+ return -EFAULT;
+ }
if (hp->interface_id != 'S') {
sg_remove_request(sfp, srp);
return -ENOSYS;
@@ -637,10 +652,13 @@
return -EMSGSIZE;
}
if ((k = verify_area(VERIFY_READ, hp->cmdp, hp->cmd_len))) {
+ sg_remove_request(sfp, srp);
+ return k; /* protects following copy_from_user()s + get_user()s */
+ }
+ if (__copy_from_user(cmnd, hp->cmdp, hp->cmd_len)) {
sg_remove_request(sfp, srp);
- return k; /* protects following copy_from_user()s + get_user()s */
+ return -EFAULT;
}
- __copy_from_user(cmnd, hp->cmdp, hp->cmd_len);
if (read_only &&
(! sg_allow_access(cmnd[0], sfp->parentdp->device->type))) {
sg_remove_request(sfp, srp);
@@ -923,8 +941,8 @@
}
}
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- __copy_to_user((void *)arg, rinfo, SZ_SG_REQ_INFO * SG_MAX_QUEUE);
- return 0;
+ return (__copy_to_user((void *)arg, rinfo, SZ_SG_REQ_INFO * SG_MAX_QUEUE))
+ ? -EFAULT : 0;
}
case SG_EMULATED_HOST:
if (sdp->detached)
@@ -972,7 +990,8 @@
unsigned char opcode = WRITE_6;
Scsi_Ioctl_Command * siocp = (void *)arg;
- copy_from_user(&opcode, siocp->data, 1);
+ if (copy_from_user(&opcode, siocp->data, 1))
+ return -EFAULT;
if (! sg_allow_access(opcode, sdp->device->type))
return -EPERM;
}
@@ -1897,7 +1916,8 @@
res = sg_u_iovec(hp, iovec_count, j, 1, &usglen, &up);
if (res) return res;
usglen = (num_xfer > usglen) ? usglen : num_xfer;
- __copy_from_user(p, up, usglen);
+ if (__copy_from_user(p, up, usglen))
+ return -EFAULT;
p += usglen;
num_xfer -= usglen;
if (num_xfer <= 0)
@@ -1925,20 +1945,28 @@
break;
if (ksglen > usglen) {
if (usglen >= num_xfer) {
- if (ok) __copy_from_user(p, up, num_xfer);
+ if (ok)
+ if (__copy_from_user(p, up, num_xfer))
+ return -EFAULT;
return 0;
}
- if (ok) __copy_from_user(p, up, usglen);
+ if (ok)
+ if (__copy_from_user(p, up, usglen))
+ return -EFAULT;
p += usglen;
ksglen -= usglen;
break;
}
else {
if (ksglen >= num_xfer) {
- if (ok) __copy_from_user(p, up, num_xfer);
+ if (ok)
+ if (__copy_from_user(p, up, num_xfer))
+ return -EFAULT;
return 0;
}
- if (ok) __copy_from_user(p, up, ksglen);
+ if (ok)
+ if (__copy_from_user(p, up, ksglen))
+ return -EFAULT;
up += ksglen;
usglen -= ksglen;
}
@@ -1964,9 +1992,10 @@
count = num_xfer;
}
else {
- __copy_from_user(&u_iovec,
- (unsigned char *)hp->dxferp + (ind * SZ_SG_IOVEC),
- SZ_SG_IOVEC);
+ if (__copy_from_user(&u_iovec,
+ (unsigned char *)hp->dxferp + (ind * SZ_SG_IOVEC),
+ SZ_SG_IOVEC))
+ return -EFAULT;
p = (unsigned char *)u_iovec.iov_base;
count = (int)u_iovec.iov_len;
}
@@ -2050,7 +2079,8 @@
res = sg_u_iovec(hp, iovec_count, j, 0, &usglen, &up);
if (res) return res;
usglen = (num_xfer > usglen) ? usglen : num_xfer;
- __copy_to_user(up, p, usglen);
+ if (__copy_to_user(up, p, usglen))
+ return -EFAULT;
p += usglen;
num_xfer -= usglen;
if (num_xfer <= 0)
@@ -2078,20 +2108,28 @@
break;
if (ksglen > usglen) {
if (usglen >= num_xfer) {
- if (ok) __copy_to_user(up, p, num_xfer);
+ if (ok)
+ if (__copy_to_user(up, p, num_xfer))
+ return -EFAULT;
return 0;
}
- if (ok) __copy_to_user(up, p, usglen);
+ if (ok)
+ if (__copy_to_user(up, p, usglen))
+ return -EFAULT;
p += usglen;
ksglen -= usglen;
break;
}
else {
if (ksglen >= num_xfer) {
- if (ok) __copy_to_user(up, p, num_xfer);
+ if (ok)
+ if (__copy_to_user(up, p, num_xfer))
+ return -EFAULT;
return 0;
}
- if (ok) __copy_to_user(up, p, ksglen);
+ if (ok)
+ if (__copy_to_user(up, p, ksglen))
+ return -EFAULT;
up += ksglen;
usglen -= ksglen;
}
@@ -2101,14 +2139,14 @@
return 0;
}
-static void sg_read_oxfer(Sg_request * srp, char * outp, int num_read_xfer)
+static int sg_read_oxfer(Sg_request * srp, char * outp, int num_read_xfer)
{
Sg_scatter_hold * schp = &srp->data;
SCSI_LOG_TIMEOUT(4, printk("sg_read_oxfer: num_read_xfer=%dn",
num_read_xfer));
if ((! outp) || (num_read_xfer <= 0))
- return;
+ return 0;
if(schp->k_use_sg > 0) {
int k, num;
struct scatterlist * sclp = (struct scatterlist *)schp->buffer;
@@ -2116,11 +2154,13 @@
for (k = 0; (k < schp->k_use_sg) && sg_scatg2virt(sclp); ++k, ++sclp) {
num = (int)sclp->length;
if (num > num_read_xfer) {
- __copy_to_user(outp, sg_scatg2virt(sclp), num_read_xfer);
+ if (__copy_to_user(outp, sg_scatg2virt(sclp), num_read_xfer))
+ return -EFAULT;
break;
}
else {
- __copy_to_user(outp, sg_scatg2virt(sclp), num);
+ if (__copy_to_user(outp, sg_scatg2virt(sclp), num))
+ return -EFAULT;
num_read_xfer -= num;
if (num_read_xfer <= 0)
break;
@@ -2129,7 +2169,9 @@
}
}
else
- __copy_to_user(outp, schp->buffer, num_read_xfer);
+ if (__copy_to_user(outp, schp->buffer, num_read_xfer))
+ return -EFAULT;
+ return 0;
}
static void sg_build_reserve(Sg_fd * sfp, int req_size)
@@ -2820,7 +2862,9 @@
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
num = (count < 10) ? count : 10;
- copy_from_user(buff, buffer, num);
+ if (copy_from_user(buff, buffer, num)) {
+ return -EFAULT;
+ }
buff[num] = '0';
sg_allow_dio = simple_strtoul(buff, 0, 10) ? 1 : 0;
return count;
@@ -2847,7 +2891,8 @@
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
num = (count < 10) ? count : 10;
- copy_from_user(buff, buffer, num);
+ if (copy_from_user(buff, buffer, num))
+ return -EFAULT;
buff[num] = '0';
k = simple_strtoul(buff, 0, 10);
if (k <= 1048576) {
--
Don't blame me: the Monkey is driving
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [TRIVIAL] copy_XX_user for sg.c - take 2
2002-07-15 8:19 [TRIVIAL] copy_XX_user for sg.c - take 2 Rusty Trivial Russell
@ 2002-07-17 3:25 ` Douglas Gilbert
2002-07-18 1:59 ` Rusty Russell
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Gilbert @ 2002-07-17 3:25 UTC (permalink / raw)
To: Rusty Trivial Russell; +Cc: linux-scsi, wstinson
Rusty Trivial Russell wrote:
>
> [ This one is hardly trivial: it seems correct, but... Please check ]
>
> From: (via Rusty) william stinson <wstinson@infonie.fr>
>
> Hi Rusty,
>
> here is version 2 of my sg.c patch to correct indentation to 4blanks as you pointed out (I hope I didn't miss anything).
>
> This proposed patch for the sg.c driver which allows user process control of SCSI devices
> 1) checks the result of copy_XX_user
> 2) returns -EFAULT in case not all data was copied
> 3) releases resources in case of failure
> 4) return value of sg_read_oxfer changed from void to int
> in order to be able to pass back error conditions
Rusty,
This patch, together with one to disable kiobufs usage
(for Andrew Morton's direct-to-BIO changes), are in
lk 2.5.26
Doug Gilbert
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [TRIVIAL] copy_XX_user for sg.c - take 2
2002-07-17 3:25 ` Douglas Gilbert
@ 2002-07-18 1:59 ` Rusty Russell
0 siblings, 0 replies; 3+ messages in thread
From: Rusty Russell @ 2002-07-18 1:59 UTC (permalink / raw)
To: Douglas Gilbert; +Cc: linux-scsi, wstinson
In message <3D34E3BA.2B2EE2AC@torque.net> you write:
> > here is version 2 of my sg.c patch to correct indentation to 4blanks as y
ou pointed out (I hope I didn't miss anything).
>
> Rusty,
> This patch, together with one to disable kiobufs usage
> (for Andrew Morton's direct-to-BIO changes), are in
> lk 2.5.26
>
> Doug Gilbert
Hi Doug,
I have automated scripts which closes the trivial patch and
mail out when this happens (I get about 20 patches a week through the
trivial patch system at the moment, so I need that). Indeed, it sent
out the following when 2.5.26 was released:
================
From: Trivial Patch Monkey <trivial@rustcorp.com.au>
Subject: Re: [PATCH] copy_XX_user for sg.c - take 2: Patch is closed.
To: william stinson <wstinson@infonie.fr>
Patch is closed.
Status of this patch:
2.5: 2.5.26: Some parts already applied?
FYI: You can put "No Status Update" somewhere in your mail body before
the patch to disable status tracking (you will still get an initial ack).
Complaints etc. to trivial-feedback@rustcorp.com.au.
================
But thanks for the confirmation!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2002-07-18 1:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-07-15 8:19 [TRIVIAL] copy_XX_user for sg.c - take 2 Rusty Trivial Russell
2002-07-17 3:25 ` Douglas Gilbert
2002-07-18 1:59 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox