Messages in this thread Patch in this message | | | Date | Mon, 28 Aug 2000 16:17:40 -0300 | From | Arnaldo Carvalho de Melo <> | 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/
| |