Messages in this thread Patch in this message | | | Date | Tue, 17 Oct 2000 17:59:22 -0200 | From | Arnaldo Carvalho de Melo <> | Subject | [PATCH] cpqarray: several fixes/cleanups |
| |
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/
| |