QIC02 driver (patch correction)

Dirk Weigenand (weigo@wilma.rz.uni-leipzig.de)
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