[PATCH] cpqarray: bugfixes & cleanups

From: Arnaldo Carvalho de Melo (acme@conectiva.com.br)
Date: Mon Aug 28 2000 - 14:17:40 EST


Hi,
        
        Please consider applying, comments are on the patch.

                        - Arnaldo

--- linux-2.4.0-test7/drivers/block/cpqarray.c Thu Jul 6 23:25:21 2000
+++ linux-2.4.0-test7.acme/drivers/block/cpqarray.c Mon Aug 28 16:05:20 2000
@@ -21,6 +21,13 @@
  * If you want to make changes, improve or add functionality to this
  * driver, you'll probably need the Compaq Array Controller Interface
  * Specificiation (Document number ECG086/1198)
+ *
+ * Changes:
+ * Arnaldo Carvalho de Melo <acme@conectiva.com.br> - 2000/08/28
+ * - check cmd_alloc return in sendcmd
+ * - s/suser/capable/
+ * - check put_user, copy_.*_user return
+ * - some cleanups
  */
 #include <linux/config.h> /* CONFIG_PROC_FS */
 #include <linux/module.h>
@@ -376,7 +383,7 @@
         cpqarray_eisa_detect();
         
         if (nr_ctlr == 0)
- return(num_cntlrs_reg);
+ return 0;
 
         printk(DRIVER_NAME "\n");
         printk("Found %d controller(s)\n", nr_ctlr);
@@ -384,37 +391,19 @@
         /* allocate space for disk structs */
         ida = kmalloc(sizeof(struct hd_struct)*nr_ctlr*NWD*16, GFP_KERNEL);
         if(ida==NULL)
- {
- printk( KERN_ERR "cpqarray: out of memory");
- return(num_cntlrs_reg);
- }
+ goto cleanup_msg;
         
         ida_sizes = kmalloc(sizeof(int)*nr_ctlr*NWD*16, GFP_KERNEL);
         if(ida_sizes==NULL)
- {
- kfree(ida);
- printk( KERN_ERR "cpqarray: out of memory");
- return(num_cntlrs_reg);
- }
+ goto cleanup_ida;
 
         ida_blocksizes = kmalloc(sizeof(int)*nr_ctlr*NWD*16, GFP_KERNEL);
         if(ida_blocksizes==NULL)
- {
- kfree(ida);
- kfree(ida_sizes);
- printk( KERN_ERR "cpqarray: out of memory");
- return(num_cntlrs_reg);
- }
+ goto cleanup_sizes;
 
         ida_hardsizes = kmalloc(sizeof(int)*nr_ctlr*NWD*16, GFP_KERNEL);
         if(ida_hardsizes==NULL)
- {
- kfree(ida);
- kfree(ida_sizes);
- kfree(ida_blocksizes);
- printk( KERN_ERR "cpqarray: out of memory");
- return(num_cntlrs_reg);
- }
+ goto cleanup_block;
 
         memset(ida, 0, sizeof(struct hd_struct)*nr_ctlr*NWD*16);
         memset(ida_sizes, 0, sizeof(int)*nr_ctlr*NWD*16);
@@ -462,7 +451,6 @@
                         free_irq(hba[i]->intr, hba[i]);
                         unregister_blkdev(MAJOR_NR+i, hba[i]->devname);
                         num_cntlrs_reg--;
- printk( KERN_ERR "cpqarray: out of memory");
 
                         /* If num_cntlrs_reg == 0, no controllers worked.
                          * init_module will fail, so clean up global
@@ -470,12 +458,8 @@
                         */
         
                         if (num_cntlrs_reg == 0)
- {
- kfree(ida);
- kfree(ida_sizes);
- kfree(ida_hardsizes);
- kfree(ida_blocksizes);
- }
+ goto cleanup_hard;
+ printk( KERN_ERR "cpqarray: out of memory");
                         return(num_cntlrs_reg);
         
                 }
@@ -525,6 +509,12 @@
         }
         /* done ! */
         return(num_cntlrs_reg);
+cleanup_hard: kfree(ida_hardsizes);
+cleanup_block: kfree(ida_blocksizes);
+cleanup_sizes: kfree(ida_sizes);
+cleanup_ida: kfree(ida);
+cleanup_msg: printk(KERN_ERR "cpqarray: out of memory\n");
+ return 0;
 }
 
 /*
@@ -797,7 +787,7 @@
         if (ctlr > MAX_CTLR || hba[ctlr] == NULL)
                 return -ENXIO;
 
- if (!suser() && ida_sizes[(ctlr << CTLR_SHIFT) +
+ if (!capable(CAP_SYS_ADMIN) && ida_sizes[(ctlr << CTLR_SHIFT) +
                                                 MINOR(inode->i_rdev)] == 0)
                 return -ENXIO;
 
@@ -807,7 +797,7 @@
          * but I'm already using way to many device nodes to claim another one
          * for "raw controller".
          */
- if (suser()
+ if (capable(CAP_SYS_ADMIN)
                 && ida_sizes[(ctlr << CTLR_SHIFT) + MINOR(inode->i_rdev)] == 0
                 && MINOR(inode->i_rdev) != 0)
                 return -ENXIO;
@@ -1157,37 +1147,34 @@
                         diskinfo[1] = 0x3f;
                         diskinfo[2] = hba[ctlr]->drv[dsk].nr_blks / (0xff*0x3f);
                 }
- put_user(diskinfo[0], &geo->heads);
- put_user(diskinfo[1], &geo->sectors);
- put_user(diskinfo[2], &geo->cylinders);
- put_user(ida[(ctlr<<CTLR_SHIFT)+MINOR(inode->i_rdev)].start_sect, &geo->start);
+ if (put_user(diskinfo[0], &geo->heads) ||
+ put_user(diskinfo[1], &geo->sectors) ||
+ put_user(diskinfo[2], &geo->cylinders) ||
+ put_user(ida[(ctlr<<CTLR_SHIFT)+MINOR(inode->i_rdev)].start_sect, &geo->start))
+ return -EFAULT;
                 return 0;
         case IDAGETDRVINFO:
- return copy_to_user(&io->c.drv,&hba[ctlr]->drv[dsk],sizeof(drv_info_t));
+ return copy_to_user(&io->c.drv,&hba[ctlr]->drv[dsk],sizeof(drv_info_t)) ? -EFAULT : 0;
         case BLKGETSIZE:
                 if (!arg) return -EINVAL;
- put_user(ida[(ctlr<<CTLR_SHIFT)+MINOR(inode->i_rdev)].nr_sects, (long*)arg);
- return 0;
+ return put_user(ida[(ctlr<<CTLR_SHIFT)+MINOR(inode->i_rdev)].nr_sects, (long*)arg);
         case BLKRRPART:
                 return revalidate_logvol(inode->i_rdev, 1);
         case IDAPASSTHRU:
- if (!suser()) return -EPERM;
- error = copy_from_user(&my_io, io, sizeof(my_io));
- if (error) return error;
+ if (!capable(CAP_SYS_ADMIN)) return -EPERM;
+ if (copy_from_user(&my_io, io, sizeof(my_io)))
+ return -EFAULT;
                 error = ida_ctlr_ioctl(ctlr, dsk, &my_io);
                 if (error) return error;
- error = copy_to_user(io, &my_io, sizeof(my_io));
- return error;
+ return copy_to_user(io, &my_io, sizeof(my_io)) ? -EFAULT : 0;
         case IDAGETCTLRSIG:
                 if (!arg) return -EINVAL;
- put_user(hba[ctlr]->ctlr_sig, (int*)arg);
- return 0;
+ return put_user(hba[ctlr]->ctlr_sig, (int*)arg);
         case IDAREVALIDATEVOLS:
                 return revalidate_allvol(inode->i_rdev);
         case IDADRIVERVERSION:
                 if (!arg) return -EINVAL;
- put_user(DRIVER_VERSION, (unsigned long*)arg);
- return 0;
+ return put_user(DRIVER_VERSION, (unsigned long*)arg);
 
         case BLKFLSBUF:
         case BLKROSET:
@@ -1196,11 +1183,9 @@
         case BLKRAGET:
         case BLKPG:
                 return blk_ioctl(inode->i_rdev, cmd, arg);
-
- default:
- return -EBADRQC;
         }
-
+
+ return -EBADRQC;
 }
 /*
  * ida_ctlr_ioctl is for passing commands to the controller from userspace.
@@ -1218,7 +1203,7 @@
         cmdlist_t *c;
         void *p = NULL;
         unsigned long flags;
- int error;
+ int error = -ENOMEM;
 
         if ((c = cmd_alloc(NULL)) == NULL)
                 return -ENOMEM;
@@ -1237,12 +1222,11 @@
         case PASSTHRU_A:
                 p = kmalloc(io->sg[0].size, GFP_KERNEL);
                 if (!p)
- {
- error = -ENOMEM;
- cmd_free(NULL, c);
- return(error);
+ goto cleanup_cmd;
+ if (copy_from_user(p, (void*)io->sg[0].addr, io->sg[0].size)) {
+ error = -EFAULT;
+ goto cleanup_p;
                 }
- copy_from_user(p, (void*)io->sg[0].addr, io->sg[0].size);
                 c->req.hdr.blk = virt_to_bus(&(io->c));
                 c->req.sg[0].size = io->sg[0].size;
                 c->req.sg[0].addr = virt_to_bus(p);
@@ -1251,12 +1235,7 @@
         case IDA_READ:
                 p = kmalloc(io->sg[0].size, GFP_KERNEL);
                 if (!p)
- {
- error = -ENOMEM;
- cmd_free(NULL, c);
- return(error);
- }
-
+ goto cleanup_cmd;
                 c->req.sg[0].size = io->sg[0].size;
                 c->req.sg[0].addr = virt_to_bus(p);
                 c->req.hdr.sg_cnt = 1;
@@ -1266,12 +1245,11 @@
         case DIAG_PASS_THRU:
                 p = kmalloc(io->sg[0].size, GFP_KERNEL);
                 if (!p)
- {
- error = -ENOMEM;
- cmd_free(NULL, c);
- return(error);
- }
- copy_from_user(p, (void*)io->sg[0].addr, io->sg[0].size);
+ goto cleanup_cmd;
+ if (copy_from_user(p, (void*)io->sg[0].addr, io->sg[0].size)) {
+ error = -EFAULT;
+ goto cleanup_p;
+ }
                 c->req.sg[0].size = io->sg[0].size;
                 c->req.sg[0].addr = virt_to_bus(p);
                 c->req.hdr.sg_cnt = 1;
@@ -1298,7 +1276,10 @@
         case PASSTHRU_A:
         case IDA_READ:
         case DIAG_PASS_THRU:
- copy_to_user((void*)io->sg[0].addr, p, io->sg[0].size);
+ if (copy_to_user((void*)io->sg[0].addr, p, io->sg[0].size)) {
+ error = -EFAULT;
+ goto cleanup_p;
+ }
                 /* fall through and free p */
         case IDA_WRITE:
         case IDA_WRITE_MEDIA:
@@ -1311,6 +1292,11 @@
         io->rcode = c->req.hdr.rcode;
         cmd_free(NULL, c);
         return(0);
+cleanup_p:
+ kfree(p);
+cleanup_cmd:
+ cmd_free(NULL, c);
+ return error;
 }
 
 /*
@@ -1378,6 +1364,8 @@
         ctlr_info_t *info_p = hba[ctlr];
 
         c = cmd_alloc(info_p);
+ if (!c)
+ return IO_ERROR;
         c->ctlr = ctlr;
         c->hdr.unit = log_unit;
         c->hdr.prio = 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:22 EST