Commit a16a4741 authored by Al Viro's avatar Al Viro Committed by Martin K. Petersen

scsi: sg: sg_ioctl(): fix copyout handling

First of all, __put_user() can fail with access_ok() succeeding.  And
access_ok() + __copy_to_user() is spelled copy_to_user()...

__put_user() *can* fail with access_ok() succeeding...

Link: https://lore.kernel.org/r/20191017193925.25539-1-viro@ZenIV.linux.org.ukSigned-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Acked-by: default avatarDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent ec990306
...@@ -963,26 +963,21 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) ...@@ -963,26 +963,21 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
case SG_GET_LOW_DMA: case SG_GET_LOW_DMA:
return put_user((int) sdp->device->host->unchecked_isa_dma, ip); return put_user((int) sdp->device->host->unchecked_isa_dma, ip);
case SG_GET_SCSI_ID: case SG_GET_SCSI_ID:
if (!access_ok(p, sizeof (sg_scsi_id_t))) {
return -EFAULT; sg_scsi_id_t v;
else {
sg_scsi_id_t __user *sg_idp = p;
if (atomic_read(&sdp->detaching)) if (atomic_read(&sdp->detaching))
return -ENODEV; return -ENODEV;
__put_user((int) sdp->device->host->host_no, memset(&v, 0, sizeof(v));
&sg_idp->host_no); v.host_no = sdp->device->host->host_no;
__put_user((int) sdp->device->channel, v.channel = sdp->device->channel;
&sg_idp->channel); v.scsi_id = sdp->device->id;
__put_user((int) sdp->device->id, &sg_idp->scsi_id); v.lun = sdp->device->lun;
__put_user((int) sdp->device->lun, &sg_idp->lun); v.scsi_type = sdp->device->type;
__put_user((int) sdp->device->type, &sg_idp->scsi_type); v.h_cmd_per_lun = sdp->device->host->cmd_per_lun;
__put_user((short) sdp->device->host->cmd_per_lun, v.d_queue_depth = sdp->device->queue_depth;
&sg_idp->h_cmd_per_lun); if (copy_to_user(p, &v, sizeof(sg_scsi_id_t)))
__put_user((short) sdp->device->queue_depth, return -EFAULT;
&sg_idp->d_queue_depth);
__put_user(0, &sg_idp->unused[0]);
__put_user(0, &sg_idp->unused[1]);
return 0; return 0;
} }
case SG_SET_FORCE_PACK_ID: case SG_SET_FORCE_PACK_ID:
...@@ -992,20 +987,16 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) ...@@ -992,20 +987,16 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
sfp->force_packid = val ? 1 : 0; sfp->force_packid = val ? 1 : 0;
return 0; return 0;
case SG_GET_PACK_ID: case SG_GET_PACK_ID:
if (!access_ok(ip, sizeof (int)))
return -EFAULT;
read_lock_irqsave(&sfp->rq_list_lock, iflags); read_lock_irqsave(&sfp->rq_list_lock, iflags);
list_for_each_entry(srp, &sfp->rq_list, entry) { list_for_each_entry(srp, &sfp->rq_list, entry) {
if ((1 == srp->done) && (!srp->sg_io_owned)) { if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock, read_unlock_irqrestore(&sfp->rq_list_lock,
iflags); iflags);
__put_user(srp->header.pack_id, ip); return put_user(srp->header.pack_id, ip);
return 0;
} }
} }
read_unlock_irqrestore(&sfp->rq_list_lock, iflags); read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
__put_user(-1, ip); return put_user(-1, ip);
return 0;
case SG_GET_NUM_WAITING: case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags); read_lock_irqsave(&sfp->rq_list_lock, iflags);
val = 0; val = 0;
...@@ -1073,9 +1064,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) ...@@ -1073,9 +1064,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
val = (sdp->device ? 1 : 0); val = (sdp->device ? 1 : 0);
return put_user(val, ip); return put_user(val, ip);
case SG_GET_REQUEST_TABLE: case SG_GET_REQUEST_TABLE:
if (!access_ok(p, SZ_SG_REQ_INFO * SG_MAX_QUEUE)) {
return -EFAULT;
else {
sg_req_info_t *rinfo; sg_req_info_t *rinfo;
rinfo = kcalloc(SG_MAX_QUEUE, SZ_SG_REQ_INFO, rinfo = kcalloc(SG_MAX_QUEUE, SZ_SG_REQ_INFO,
...@@ -1085,7 +1074,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) ...@@ -1085,7 +1074,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
read_lock_irqsave(&sfp->rq_list_lock, iflags); read_lock_irqsave(&sfp->rq_list_lock, iflags);
sg_fill_request_table(sfp, rinfo); sg_fill_request_table(sfp, rinfo);
read_unlock_irqrestore(&sfp->rq_list_lock, iflags); read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
result = __copy_to_user(p, rinfo, result = copy_to_user(p, rinfo,
SZ_SG_REQ_INFO * SG_MAX_QUEUE); SZ_SG_REQ_INFO * SG_MAX_QUEUE);
result = result ? -EFAULT : 0; result = result ? -EFAULT : 0;
kfree(rinfo); kfree(rinfo);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment