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.uk
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index cce7575..6344604 100644 (file)
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -963,26 +963,21 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
        case SG_GET_LOW_DMA:
                return put_user((int) sdp->device->host->unchecked_isa_dma, ip);
        case SG_GET_SCSI_ID:
-               if (!access_ok(p, sizeof (sg_scsi_id_t)))
-                       return -EFAULT;
-               else {
-                       sg_scsi_id_t __user *sg_idp = p;
+               {
+                       sg_scsi_id_t v;
 
                        if (atomic_read(&sdp->detaching))
                                return -ENODEV;
-                       __put_user((int) sdp->device->host->host_no,
-                                  &sg_idp->host_no);
-                       __put_user((int) sdp->device->channel,
-                                  &sg_idp->channel);
-                       __put_user((int) sdp->device->id, &sg_idp->scsi_id);
-                       __put_user((int) sdp->device->lun, &sg_idp->lun);
-                       __put_user((int) sdp->device->type, &sg_idp->scsi_type);
-                       __put_user((short) sdp->device->host->cmd_per_lun,
-                                  &sg_idp->h_cmd_per_lun);
-                       __put_user((short) sdp->device->queue_depth,
-                                  &sg_idp->d_queue_depth);
-                       __put_user(0, &sg_idp->unused[0]);
-                       __put_user(0, &sg_idp->unused[1]);
+                       memset(&v, 0, sizeof(v));
+                       v.host_no = sdp->device->host->host_no;
+                       v.channel = sdp->device->channel;
+                       v.scsi_id = sdp->device->id;
+                       v.lun = sdp->device->lun;
+                       v.scsi_type = sdp->device->type;
+                       v.h_cmd_per_lun = sdp->device->host->cmd_per_lun;
+                       v.d_queue_depth = sdp->device->queue_depth;
+                       if (copy_to_user(p, &v, sizeof(sg_scsi_id_t)))
+                               return -EFAULT;
                        return 0;
                }
        case SG_SET_FORCE_PACK_ID:
@@ -992,20 +987,16 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
                sfp->force_packid = val ? 1 : 0;
                return 0;
        case SG_GET_PACK_ID:
-               if (!access_ok(ip, sizeof (int)))
-                       return -EFAULT;
                read_lock_irqsave(&sfp->rq_list_lock, iflags);
                list_for_each_entry(srp, &sfp->rq_list, entry) {
                        if ((1 == srp->done) && (!srp->sg_io_owned)) {
                                read_unlock_irqrestore(&sfp->rq_list_lock,
                                                       iflags);
-                               __put_user(srp->header.pack_id, ip);
-                               return 0;
+                               return put_user(srp->header.pack_id, ip);
                        }
                }
                read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-               __put_user(-1, ip);
-               return 0;
+               return put_user(-1, ip);
        case SG_GET_NUM_WAITING:
                read_lock_irqsave(&sfp->rq_list_lock, iflags);
                val = 0;
@@ -1073,9 +1064,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
                val = (sdp->device ? 1 : 0);
                return put_user(val, ip);
        case SG_GET_REQUEST_TABLE:
-               if (!access_ok(p, SZ_SG_REQ_INFO * SG_MAX_QUEUE))
-                       return -EFAULT;
-               else {
+               {
                        sg_req_info_t *rinfo;
 
                        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)
                        read_lock_irqsave(&sfp->rq_list_lock, iflags);
                        sg_fill_request_table(sfp, rinfo);
                        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);
                        result = result ? -EFAULT : 0;
                        kfree(rinfo);