lkml.org 
[lkml]   [2000]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] cpqarray: bugfixes & cleanups
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/
\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.029 / U:0.648 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site