[PATCH] cpqarray: several fixes/cleanups

From: Arnaldo Carvalho de Melo (acme@conectiva.com.br)
Date: Tue Oct 17 2000 - 14:59:22 EST


Jeff,

        Here it is, resubmitting after rediffing wrt 2.4.0-test10-pre3.

- Arnaldo

--- linux-2.4.0-test10-3/drivers/block/cpqarray.c Fri Oct 13 18:40:39 2000
+++ linux-2.4.0-test10-3.acme/drivers/block/cpqarray.c Tue Oct 17 11:38:53 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/10/17
+ * - s/suser/capable/
+ * - check put_user, copy_.*_user return
+ * - some cleanups
+ *
  */
 #include <linux/config.h> /* CONFIG_PROC_FS */
 #include <linux/module.h>
@@ -383,7 +390,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);
@@ -391,37 +398,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);
@@ -469,7 +458,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
@@ -477,12 +465,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);
         
                 }
@@ -532,6 +516,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;
 }
 
 /*
@@ -800,7 +790,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;
 
@@ -810,7 +800,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;
@@ -1160,37 +1150,37 @@
                         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));
+ if (copy_to_user(&io->c.drv,&hba[ctlr]->drv[dsk],sizeof(drv_info_t)))
+ return -EFAULT;
+ return 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 IDAGETPCIINFO:
         {
                 
@@ -1235,10 +1225,10 @@
         cmdlist_t *c;
         void *p = NULL;
         unsigned long flags;
- int error;
+ int error = -ENOMEM;
 
         if ((c = cmd_alloc(NULL)) == NULL)
- return -ENOMEM;
+ goto cleanup_enomem;
         c->ctlr = ctlr;
         c->hdr.unit = (io->unit & UNITVALID) ? (io->unit & ~UNITVALID) : dsk;
         c->hdr.size = sizeof(rblk_t) >> 2;
@@ -1254,12 +1244,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);
@@ -1268,12 +1257,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;
@@ -1283,12 +1267,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;
@@ -1315,7 +1298,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:
@@ -1328,6 +1314,9 @@
         io->rcode = c->req.hdr.rcode;
         cmd_free(NULL, c);
         return(0);
+cleanup_p: kfree(p);
+cleanup_cmd: cmd_free(NULL, c);
+cleanup_enomem: return error;
 }
 
 /*
-
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 : Mon Oct 23 2000 - 21:00:12 EST