Messages in this thread Patch in this message | | | Date | Sat, 24 Jan 1998 17:52:32 -0500 | From | Bill Hawes <> | Subject | patch for 2.1.81 ppp.c -- please test! |
| |
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--- 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; } | |