Messages in this thread Patch in this message |  | | From | Dirk Weigenand <> | Subject | QIC02 driver (patch correction) | Date | Mon, 21 Oct 1996 19:45:14 +0100 (GMT+0100) |
| |
Hi,
i just realized that my patch posted earlier today didn't applay cleanly to a fresh 2.1.5 tree. The remade patch is appended.
regards, Dirk
--- linux/drivers/char/tpqic02.c.orig Mon Oct 21 19:37:32 1996 +++ linux/drivers/char/tpqic02.c Mon Oct 21 19:38:44 1996 @@ -1920,7 +1920,6 @@ static long qic02_tape_read(struct inode * inode, struct file * filp, char * buf, unsigned long count) { - int error; kdev_t dev = inode->i_rdev; unsigned short flags = filp->f_flags; unsigned long bytes_todo, bytes_done, total_bytes_done = 0; @@ -1946,12 +1945,6 @@ if (status_bytes_wr) /* Once written, no more reads, 'till after WFM. */ return -EACCES; - - /* Make sure buffer is safe to write into. */ - error = verify_area(VERIFY_WRITE, buf, count); - if (error) - return error; - /* This is rather ugly because it has to implement a finite state * machine in order to handle the EOF situations properly. */ @@ -1965,7 +1958,9 @@ /* Must ensure that user program sees exactly one EOF token (==0) */ if (return_read_eof==YES) { if (TPQDBG(DEBUG)) - printk("read: return_read_eof==%d, reported_read_eof==%d, total_bytes_done==%lu\n", return_read_eof, reported_read_eof, total_bytes_done); + printk("read: return_read_eof==%d, reported_read_eof==%d, + total_bytes_done==%lu\n", return_read_eof, + reported_read_eof, total_bytes_done); if (reported_read_eof==NO) { /* have not yet returned EOF to user program */ @@ -1983,7 +1978,10 @@ * causing a sense(). Subsequent read/writes will * continue after the FM. */ -/*********** ?????????? this should check for (EOD|NDT), not EOM, 'cause we can read past EW: ************/ + /* + * ?????????? this should check for (EOD|NDT), not EOM, + * 'cause we can read past EW: + */ if (status_eom_detected) /* If EOM, nothing left to read, so keep returning EOFs. *** should probably set some flag to avoid clearing @@ -2037,13 +2035,20 @@ tpqputs(TPQD_ALWAYS, "read: Oops, read more bytes than requested"); return -EIO; } - /* copy buffer to user-space in one go */ + /* + * copy buffer to user-space in one go and test wether + * we could write all data + */ if (bytes_done>0) - copy_to_user( (void *) buf, (void *) bus_to_virt(buffaddr), bytes_done); + if (copy_to_user((void *)buf, (void *)bus_to_virt(buffaddr), + bytes_done)) + return -EFAULT; #if 1 /* Checks Ton's patch below */ if ((return_read_eof == NO) && (status_eof_detected == YES)) { - printk(TPQIC02_NAME ": read(): return_read_eof=%d, status_eof_detected=YES. return_read_eof:=YES\n", return_read_eof); + printk(TPQIC02_NAME ": read(): return_read_eof=%d, + status_eof_detected=YES. return_read_eof:=YES\n", + return_read_eof); } #endif if ((bytes_todo != bytes_done) || (status_eof_detected == YES)) @@ -2096,7 +2101,6 @@ static long qic02_tape_write(struct inode * inode, struct file * filp, const char * buf, unsigned long count) { - int error; kdev_t dev = inode->i_rdev; unsigned short flags = filp->f_flags; unsigned long bytes_todo, bytes_done, total_bytes_done = 0; @@ -2130,11 +2134,6 @@ return -EACCES; /* don't even try when write protected */ } - /* Make sure buffer is safe to read from. */ - error = verify_area(VERIFY_READ, buf, count); - if (error) - return error; - if (doing_read == YES) terminate_read(0); @@ -2167,7 +2166,9 @@ /* copy from user to DMA buffer and initiate transfer. */ if (bytes_todo>0) { - copy_from_user( (void *) bus_to_virt(buffaddr), (const void *) buf, bytes_todo); + if (copy_from_user((void *)bus_to_virt(buffaddr), + (const void *)buf, bytes_todo)) + return -EFAULT; /****************** similar problem with read() at FM could happen here at EOT. ******************/ @@ -2558,7 +2559,6 @@ unsigned int iocmd, unsigned long ioarg) { int error; - short i; int dev_maj = MAJOR(inode->i_rdev); int c; struct mtop operation; @@ -2588,9 +2588,9 @@ if (c == DDIOCSDBG) { if (!suser()) return -EPERM; - error = verify_area(VERIFY_READ, (int *) ioarg, sizeof(int)); - if (error) return error; c = get_user(sizeof(int), (int *) ioarg); + if (c == -EFAULT) + return -EFAULT; if (c==0) { QIC02_TAPE_DEBUG = 0; return 0; @@ -2614,15 +2614,11 @@ return -EFAULT; } - /* check for valid user address */ - error = verify_area(VERIFY_WRITE, (void *) ioarg, sizeof(qic02_tape_dynconf)); - if (error) - return error; - /* copy current settings to user space */ stp = (char *) &qic02_tape_dynconf; argp = (char *) ioarg; - for (i=0; i<sizeof(qic02_tape_dynconf); i++) - put_user(*stp++, argp++); + /* check for valid user address and copy current settings to user space */ + if (copy_from_user(stp, argp, sizeof(qic02_tape_dynconf))) + return -EFAULT; return 0; } else if (c == (MTIOCSETCONFIG & IOCCMD_MASK)) { @@ -2641,14 +2637,18 @@ return -EPERM; if ((doing_read!=NO) || (doing_write!=NO)) return -EBUSY; - error = verify_area(VERIFY_READ, (char *) ioarg, sizeof(qic02_tape_dynconf)); - if (error) - return error; - - /* copy struct from user space to kernel space */ + + /* + * copy struct from user space to kernel space and check + * validity of user space + */ stp = (char *) &qic02_tape_dynconf; argp = (char *) ioarg; - copy_from_user(stp, argp, sizeof(qic02_tape_dynconf)); + /* + * should probably release resources before returning EFAULT + */ + if (copy_from_user(stp, argp, sizeof(qic02_tape_dynconf))) + return -EFAULT; if (status_zombie==NO) qic02_release_resources(); /* and go zombie */ if (update_ifc_masks(qic02_tape_dynconf.ifc_type)) @@ -2672,14 +2672,12 @@ tpqputs(TPQD_ALWAYS, "sizeof(struct mtop) does not match!"); return -EFAULT; } - error = verify_area(VERIFY_READ, (char *) ioarg, sizeof(operation)); - if (error) - return error; /* copy mtop struct from user space to kernel space */ stp = (char *) &operation; argp = (char *) ioarg; - copy_from_user(stp, argp, sizeof(operation)); + if (copy_from_user(stp, argp, sizeof(operation))) + return -EFAULT; /* ---note: mt_count is signed, negative seeks must be * --- translated to seeks in opposite direction! @@ -2727,21 +2725,16 @@ return -EFAULT; } - /* check for valid user address */ - error = verify_area(VERIFY_WRITE, (void *) ioarg, sizeof(ioctl_status)); - if (error) - return error; - /* It appears (gmt(1)) that it is normal behaviour to * first set the status with MTNOP, and then to read * it out with MTIOCGET */ - /* copy results to user space */ + /* copy results to user space and check for valid user address */ stp = (char *) &ioctl_status; argp = (char *) ioarg; - for (i=0; i<sizeof(ioctl_status); i++) - put_user(*stp++, argp++); + if (copy_to_user(stp, argp, sizeof(ioctl_status))) + return -EFAULT; return 0; @@ -2755,11 +2748,6 @@ return -EFAULT; } - /* check for valid user address */ - error = verify_area(VERIFY_WRITE, (void *) ioarg, sizeof(ioctl_tell)); - if (error) - return error; - tpqputs(TPQD_IOCTLS, "MTTELL reading block address"); if ((doing_read==YES) || (doing_write==YES)) finish_rw(AR_QCMDV_TELL_BLK); @@ -2770,11 +2758,11 @@ ioctl_tell.mt_blkno = (blk_addr[3] << 16) | (blk_addr[4] << 8) | blk_addr[5]; - /* copy results to user space */ + /* copy results to user space and check for valid user address */ stp = (char *) &ioctl_tell; argp = (char *) ioarg; - for (i=0; i<sizeof(ioctl_tell); i++) - put_user(*stp++, argp++); + if (copy_to_user(stp, argp, sizeof(ioctl_tell))) + return -EFAULT; return 0; } else -- E-mail: weigo@wilma.rz.uni-leipzig.de Phone : 0341/2614768
|  |