Messages in this thread | | | From | Paul Mackerras <> | Date | Fri, 23 Mar 2001 15:34:55 +1100 (EST) | Subject | Re: PATCH against 2.4.2: TTY hangup on PPP channel corrupts kernel memory |
| |
Kevin Buhr writes:
> I didn't realize my specific hang was a peculiarity of the older > attachment style. The channel created by pushing the PPP line
I didn't realize you were talking about linux 2.4.0 and pppd 2.3.11.
> discipline onto a TTY was connected to a unit with a PPPIOCATTACH > ioctl on the TTY---this didn't really "attach" the channel; it still > had a refcnt of only one. Through the old compatibility interface, it > was possible to call ppp_asynctty_read -> ppp_channel_read -> ppp_read > on the channel's "struct ppp_file" and wait on the channel's "rwait". > If the modem hung up, "do_tty_hangup" would call "ppp_asynctty_close" > (with a reader still in "ppp_asynctty_read") and the "struct channel" > would be freed in "ppp_unregister_channel".
That's one of the main reasons why I removed the compatibility stuff. :)
> I think your analysis of how things presently are with 2.4.2 and a > modern "pppd" is correct... > > Since the new "pppd" uses an explicit PPPIOCATTCHAN / PPPIOCCONNECT > sequence, the refcnt gets bumped to 2 and stays there while the > channel is attached. So, this specific hang isn't a problem anymore > for "ppp_async.c". It's still a problem with "ppp_synctty.c", though > (when used with "pppd" 2.3.11, say). Is the compatibility stuff in > there slated for removal, too?
Yep, and we should take out the stuff in ppp_generic.c that was called by the compatibility stuff in the channels, too.
> In particular, the comment above "ppp_asynctty_close" is misleading. > It's true that the TTY layer won't call any further line discipline > entries while the "close" is executing; however, there may be > processes already sleeping in line discipline functions called before > the hangup. For example, "ppp_asynctty_close" could be called while > we sleep in the "get_user" in "ppp_channel_ioctl" (called from > "ppp_asynctty_ioctl"). Therefore, calling "PPPIOCATTACH" on an > unattached PPP-disciplined TTY could, in unlikely circumstances > (argument swapped out), lead to a crash.
Yuck. I don't see that we can protect against this without having some sort of lock in the tty structure, though. We can't protect the existence of the channel structure with a lock inside that structure. Ideally the necessary protection would be provided at the tty level.
> I assume PPPIOCATTACH (on the TTY) is deprecated in favor of > PPPIOCATTCHAN / PPPIOCCONNECT (on the "/dev/ppp" handle). Can we > eliminate "ppp_channel_ioctl" from "ppp_async.c" entirely, as in the > patch below? We're requiring people to upgrade to "pppd" 2.4.0 > anyway, and it has no need for these calls. This would give me a warm, > fuzzy feeling.
Sure, that would be fine. I'll make up a patch and send it to Linus.
Paul. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |