The attached patch makes changes to drivers/net/ppp.c to take care of the
incorrect return code from copy_xx_user problems.
I've also changed a wait loop to use the add_wait_queue idiom instead of
interruptible_sleep_on, as the test for locking a buffer may change from an
interrupt. This could lead to waiting on the queue after the buffer has
unlocked, thereby missing the wake up.
As I don't use ppp, the patch is untested beyond compilation. If you use ppp
please give it a test and report the results.
Regards,
Bill
--------------CDC059E36EB8AC99C4E45C9A
Content-Type: text/plain; charset=us-ascii; name="ppp_81-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="ppp_81-patch"
--- drivers/net/ppp.c.old Tue Dec 30 16:08:58 1997
+++ drivers/net/ppp.c Sat Jan 24 18:41:36 1998
@@ -7,7 +7,7 @@
* Dynamic PPP devices by Jim Freeman <jfree@caldera.com>.
* ppp_tty_receive ``noisy-raise-bug'' fixed by Ove Ewerlid <ewerlid@syscon.uu.se>
*
- * ==FILEVERSION 971205==
+ * ==FILEVERSION 980123==
*
* NOTE TO MAINTAINERS:
* If you modify this file at all, please set the number above to the
@@ -2069,15 +2069,16 @@
struct ppp *ppp = tty2ppp (tty);
__u8 *new_data;
int error;
+ struct wait_queue wait = {current, NULL};
/*
* Verify the pointers.
*/
+ error = -EIO;
if (!ppp)
- return -EIO;
-
+ goto out;
if (ppp->magic != PPP_MAGIC)
- return -EIO;
+ goto out;
CHECK_PPP (-ENXIO);
/*
@@ -2104,35 +2105,42 @@
/*
* Retrieve the user's buffer
*/
- error = copy_from_user(new_data, data, count);
- if (error) {
- kfree (new_data);
- return error;
- }
+ error = -EFAULT;
+ if (copy_from_user(new_data, data, count))
+ goto out_free;
/*
- * lock this PPP unit so we will be the only writer;
- * sleep if necessary
+ * Lock this PPP unit so we will be the only writer,
+ * sleeping if necessary.
+ *
+ * Note that we add our task to the wait queue before
+ * attempting to lock, as the lock flag may be cleared
+ * from an interrupt.
*/
- while (lock_buffer (ppp->tbuf) != 0) {
+ add_wait_queue(&ppp->write_wait, &wait);
+ while (1) {
+ error = 0;
current->timeout = 0;
-#if 0
- if (ppp->flags & SC_DEBUG)
- printk (KERN_DEBUG "ppp_tty_write: sleeping\n");
-#endif
- interruptible_sleep_on (&ppp->write_wait);
+ current->state = TASK_INTERRUPTIBLE;
+ if (lock_buffer(ppp->tbuf) == 0)
+ break;
+ schedule();
+ error = -EINVAL;
ppp = tty2ppp (tty);
- if (!ppp || ppp->magic != PPP_MAGIC || !ppp->inuse
- || tty != ppp->tty) {
- kfree (new_data);
- return 0;
- }
-
- if (signal_pending(current)) {
- kfree (new_data);
- return -EINTR;
+ if (!ppp || ppp->magic != PPP_MAGIC ||
+ !ppp->inuse || tty != ppp->tty) {
+ printk("ppp_tty_write: %p invalid after wait!\n", ppp);
+ break;
}
+ error = -EINTR;
+ if (signal_pending(current))
+ break;
}
+ current->state = TASK_RUNNING;
+ remove_wait_queue(&ppp->write_wait, &wait);
+ if (error)
+ goto out_free;
+
/*
* Change the LQR frame
*/
@@ -2152,9 +2160,12 @@
} else {
ppp_dev_xmit_frame (ppp, ppp->tbuf, new_data, count);
}
+ error = count;
+out_free:
kfree (new_data);
- return count;
+out:
+ return error;
}
/*
@@ -2165,31 +2176,30 @@
ppp_set_compression (struct ppp *ppp, struct ppp_option_data *odp)
{
struct compressor *cp;
- struct ppp_option_data data;
- int error;
- int nb;
+ int error, nb;
+ unsigned long flags;
__u8 *ptr;
__u8 ccp_option[CCP_MAX_OPTION_LENGTH];
- unsigned long flags;
+ struct ppp_option_data data;
/*
* Fetch the compression parameters
*/
- error = copy_from_user(&data, odp, sizeof (data));
- if (error != 0)
- return error;
+ error = -EFAULT;
+ if (copy_from_user(&data, odp, sizeof (data)))
+ goto out;
nb = data.length;
ptr = data.ptr;
if ((__u32) nb >= (__u32)CCP_MAX_OPTION_LENGTH)
nb = CCP_MAX_OPTION_LENGTH;
- error = copy_from_user(ccp_option, ptr, nb);
- if (error != 0)
- return error;
+ if (copy_from_user(ccp_option, ptr, nb))
+ goto out;
+ error = -EINVAL;
if (ccp_option[1] < 2) /* preliminary check on the length byte */
- return (-EINVAL);
+ goto out;
save_flags(flags);
cli();
@@ -2206,53 +2216,52 @@
}
#endif /* CONFIG_KERNELD */
- if (cp != (struct compressor *) 0) {
- /*
- * Found a handler for the protocol - try to allocate
- * a compressor or decompressor.
- */
- error = 0;
- if (data.transmit) {
- if (ppp->sc_xc_state != NULL)
- (*ppp->sc_xcomp->comp_free)(ppp->sc_xc_state);
+ if (cp == NULL)
+ goto out_no_comp;
+ /*
+ * Found a handler for the protocol - try to allocate
+ * a compressor or decompressor.
+ */
+ error = 0;
+ if (data.transmit) {
+ if (ppp->sc_xc_state != NULL)
+ (*ppp->sc_xcomp->comp_free)(ppp->sc_xc_state);
- ppp->sc_xcomp = cp;
- ppp->sc_xc_state = cp->comp_alloc(ccp_option, nb);
+ ppp->sc_xcomp = cp;
+ ppp->sc_xc_state = cp->comp_alloc(ccp_option, nb);
- if (ppp->sc_xc_state == NULL) {
- if (ppp->flags & SC_DEBUG)
- printk(KERN_DEBUG "%s: comp_alloc failed\n",
- ppp->name);
- error = -ENOBUFS;
- } else {
- if (ppp->flags & SC_DEBUG)
- printk(KERN_DEBUG "%s: comp_alloc -> %p\n",
- ppp->name, ppp->sc_xc_state);
- }
- } else {
- if (ppp->sc_rc_state != NULL)
- (*ppp->sc_rcomp->decomp_free)(ppp->sc_rc_state);
- ppp->sc_rcomp = cp;
- ppp->sc_rc_state = cp->decomp_alloc(ccp_option, nb);
- if (ppp->sc_rc_state == NULL) {
- if (ppp->flags & SC_DEBUG)
- printk(KERN_DEBUG "%s: decomp_alloc failed\n",
+ if (ppp->sc_xc_state == NULL) {
+ if (ppp->flags & SC_DEBUG)
+ printk(KERN_DEBUG "%s: comp_alloc failed\n",
ppp->name);
- error = -ENOBUFS;
- } else {
- if (ppp->flags & SC_DEBUG)
- printk(KERN_DEBUG "%s: decomp_alloc -> %p\n",
- ppp->name, ppp->sc_rc_state);
- }
- }
- return (error);
+ error = -ENOBUFS;
+ } else if (ppp->flags & SC_DEBUG)
+ printk(KERN_DEBUG "%s: comp_alloc -> %p\n",
+ ppp->name, ppp->sc_xc_state);
+ } else {
+ if (ppp->sc_rc_state != NULL)
+ (*ppp->sc_rcomp->decomp_free)(ppp->sc_rc_state);
+ ppp->sc_rcomp = cp;
+ ppp->sc_rc_state = cp->decomp_alloc(ccp_option, nb);
+ if (ppp->sc_rc_state == NULL) {
+ if (ppp->flags & SC_DEBUG)
+ printk(KERN_DEBUG "%s: decomp_alloc failed\n",
+ ppp->name);
+ error = -ENOBUFS;
+ } else if (ppp->flags & SC_DEBUG)
+ printk(KERN_DEBUG "%s: decomp_alloc -> %p\n",
+ ppp->name, ppp->sc_rc_state);
}
+out:
+ return error;
+out_no_comp:
+ error = -EINVAL; /* no handler found */
if (ppp->flags & SC_DEBUG)
printk(KERN_DEBUG "%s: no compressor for [%x %x %x], %x\n",
- ppp->name, ccp_option[0], ccp_option[1],
- ccp_option[2], nb);
- return (-EINVAL); /* no handler found */
+ ppp->name, ccp_option[0], ccp_option[1],
+ ccp_option[2], nb);
+ goto out;
}
/*
@@ -2409,16 +2418,20 @@
/* change absolute times to relative times. */
cur_ddinfo.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
cur_ddinfo.recv_idle = (jiffies - ppp->last_recv) / HZ;
- error = copy_to_user((void *) param3, &cur_ddinfo,
- sizeof (cur_ddinfo));
+ error = -EFAULT;
+ if (!copy_to_user((void *) param3, &cur_ddinfo,
+ sizeof (cur_ddinfo)))
+ error = 0;
}
break;
/*
* Retrieve the extended async map
*/
case PPPIOCGXASYNCMAP:
- error = copy_to_user((void *) param3, ppp->xmit_async_map,
- sizeof (ppp->xmit_async_map));
+ error = -EFAULT;
+ if (!copy_to_user((void *) param3, ppp->xmit_async_map,
+ sizeof (ppp->xmit_async_map)))
+ error = 0;
break;
/*
* Set the async extended map
@@ -2427,9 +2440,9 @@
{
__u32 temp_tbl[8];
- error = copy_from_user(temp_tbl, (void *) param3,
- sizeof (temp_tbl));
- if (error != 0)
+ error = -EFAULT;
+ if (copy_from_user(temp_tbl, (void *) param3,
+ sizeof (temp_tbl)))
break;
temp_tbl[1] = 0x00000000;
@@ -2486,9 +2499,9 @@
{
struct npioctl npi;
- error = copy_from_user(&npi, (void *) param3,
- sizeof (npi));
- if (error != 0)
+ error = -EFAULT;
+ if (copy_from_user(&npi, (void *) param3,
+ sizeof (npi)))
break;
switch (npi.protocol) {
@@ -2659,8 +2672,9 @@
/*
* Move the version data
*/
- error = copy_to_user(result, szVersion, len);
-
+ error = -EFAULT;
+ if (!copy_to_user(result, szVersion, len))
+ error = 0;
return error;
}
@@ -2695,8 +2709,9 @@
result = (struct ppp_stats *) ifr->ifr_ifru.ifru_data;
- error = copy_to_user(result, &temp, sizeof (temp));
-
+ error = -EFAULT;
+ if (!copy_to_user(result, &temp, sizeof (temp)))
+ error = 0;
return error;
}
@@ -2727,8 +2742,9 @@
*/
result = (struct ppp_comp_stats *) ifr->ifr_ifru.ifru_data;
- error = copy_to_user(result, &temp, sizeof (temp));
-
+ error = -EFAULT;
+ if (!copy_to_user(result, &temp, sizeof (temp)))
+ error = 0;
return error;
}
--------------CDC059E36EB8AC99C4E45C9A--