aboutsummaryrefslogtreecommitdiff
path: root/drivers/char/tty_ldisc.c
AgeCommit message (Collapse)Author
2009-10-03tty: Avoid dropping ldisc_mutex over hangup tty re-initializationLinus Torvalds
A couple of people have hit the WARN_ON() in drivers/char/tty_io.c, tty_open() that is unhappy about seeing the tty line discipline go away during the tty hangup. See for example http://bugzilla.kernel.org/show_bug.cgi?id=14255 and the reason is that we do the tty_ldisc_halt() outside the ldisc_mutex in order to be able to flush the scheduled work without a deadlock with vhangup_work. However, it turns out that we can solve this particular case by - using "cancel_delayed_work_sync()" in tty_ldisc_halt(), which waits for just the particular work, rather than synchronizing with any random outstanding pending work. This won't deadlock, since the buf.work we synchronize with doesn't care about the ldisc_mutex, it just flushes the tty ldisc buffers. - realize that for this particular case, we don't need to wait for any hangup work, because we are inside the hangup codepaths ourselves. so as a result we can just drop the flush_scheduled_work() entirely, and then move the tty_ldisc_halt() call to inside the mutex. That way we never expose the partially torn down ldisc state to tty_open(), and hold the ldisc_mutex over the whole sequence. Reported-by: Ingo Molnar <mingo@elte.hu> Reported-by: Heinz Diehl <htd@fancy-poultry.org> Cc: stable@kernel.org Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-09-19tty-ldisc: get rid of tty_ldisc_try_get() helper functionLinus Torvalds
Now that the /proc/tty/ldiscs handling doesn't play games with 'struct ldisc' any more, the only remaining user of 'tty_ldisc_try_get()' is 'tty_ldisc_get()' (note the lack of 'try'). And we're actually much better off folding the logic directly into that file, since the 'try' part was always about trying to get the ldisc operations, not the ldisc itself: and making that explicit inside of 'tty_ldisc_get()' clarifies the whole semantics. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>, Tested-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2009-09-19tty-ldisc: make /proc/tty/ldiscs use ldisc_ops instead of ldiscsLinus Torvalds
The /proc/tty/ldiscs file is totally and utterly un-interested in the "struct tty_ldisc" structures, and only cares about the underlying ldisc operations. So don't make it create a dummy 'struct ldisc' only to get a pointer to the operations, and then destroy it. Instead, we split up the function 'tty_ldisc_try_get()', and create a 'get_ldops()' helper that just looks up the ldisc operations based on the ldisc number. That makes the code simpler to read (smaller and more well-defined helper functions), and allows the /proc functions to avoid creating that useless dummy only to immediately free it again. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2009-08-25tty: make sure to flush any pending work when halting the ldiscLinus Torvalds
When I rewrote tty ldisc code to use proper reference counts (commits 65b770468e98 and cbe9352fa08f) in order to avoid a race with hangup, the test-program that Eric Biederman used to trigger the original problem seems to have exposed another long-standing bug: the hangup code did the 'tty_ldisc_halt()' to stop any buffer flushing activity, but unlike the other call sites it never actually flushed any pending work. As a result, if you get just the right timing, the pending work may be just about to execute (ie the timer has already triggered and thus cancel_delayed_work() was a no-op), when we then re-initialize the ldisc from under it. That, in turn, results in various random problems, usually seen as a NULL pointer dereference in run_timer_softirq() or a BUG() in worker_thread (but it can be almost anything). Fix it by adding the required 'flush_scheduled_work()' after doing the tty_ldisc_halt() (this also requires us to move the ldisc halt to before taking the ldisc mutex in order to avoid a deadlock with the workqueue executing do_tty_hangup, which requires the mutex). The locking should be cleaned up one day (the requirement to do this outside the ldisc_mutex is very annoying, and weakens the lock), but that's a larger and separate undertaking. Reported-by: Eric W. Biederman <ebiederm@xmission.com> Tested-by: Xiaotian Feng <xtfeng@gmail.com> Tested-by: Yanmin Zhang <yanmin_zhang@linux.intel.com> Tested-by: Dave Young <hidave.darkstar@gmail.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Greg Kroah-Hartman <gregkh@suse.de> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-08-04tty-ldisc: be more careful in 'put_ldisc' lockingLinus Torvalds
Use 'atomic_dec_and_lock()' to make sure that we always hold the tty_ldisc_lock when the ldisc count goes to zero. That way we can never race against 'tty_ldisc_try()' increasing the count again. Reported-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2009-08-04tty-ldisc: turn ldisc user count into a proper refcountLinus Torvalds
By using the user count for the actual lifetime rules, we can get rid of the silly "wait_for_idle" logic, because any busy ldisc will automatically stay around until the last user releases it. This avoids a host of odd issues, and simplifies the code. So now, when the last ldisc reference is dropped, we just release the ldisc operations struct reference, and free the ldisc. It looks obvious enough, and it does work for me, but the counting _could_ be off. It probably isn't (bad counting in the new version would generally imply that the old code did something really bad, like free an ldisc with a non-zero count), but it does need some testing, and preferably somebody looking at it. With this change, both 'tty_ldisc_put()' and 'tty_ldisc_deref()' are just aliases for the new ref-counting 'put_ldisc()'. Both of them decrement the ldisc user count and free it if it goes down to zero. They're identical functions, in other words. But the reason they still exist as sepate functions is that one of them was exported (tty_ldisc_deref) and had a stupid name (so I don't want to use it as the main name), and the other one was used in multiple places (and I didn't want to make the patch larger just to rename the users). In addition to the refcounting, I did do some minimal cleanup. For example, now "tty_ldisc_try()" actually returns the ldisc it got under the lock, rather than returning true/false and then the caller would look up the ldisc again (now without the protection of the lock). That said, there's tons of dubious use of 'tty->ldisc' without obviously proper locking or refcounting left. I expressly did _not_ want to try to fix it all, keeping the patch minimal. There may or may not be bugs in that kind of code, but they wouldn't be _new_ bugs. That said, even if the bugs aren't new, the timing and lifetime will change. For example, some silly code may depend on the 'tty->ldisc' pointer not changing because they hold a refcount on the 'ldisc'. And that's no longer true - if you hold a ref on the ldisc, the 'ldisc' itself is safe, but tty->ldisc may change. So the proper locking (remains) to hold tty->ldisc_mutex if you expect tty->ldisc to be stable. That's not really a _new_ rule, but it's an example of something that the old code might have unintentionally depended on and hidden bugs. Whatever. The patch _looks_ sensible to me. The only users of ldisc->users are: - get_ldisc() - atomically increment the count - put_ldisc() - atomically decrements the count and releases if zero - tty_ldisc_try_get() - creates the ldisc, and sets the count to 1. The ldisc should then either be released, or be attached to a tty. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Tested-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by> Acked-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2009-08-04tty-ldisc: make refcount be atomic_t 'users' countLinus Torvalds
This is pure preparation of changing the ldisc reference counting to be a true refcount that defines the lifetime of the ldisc. But this is a purely syntactic change for now to make the next steps easier. This patch should make no semantic changes at all. But I wanted to make the ldisc refcount be an atomic (I will be touching it without locks soon enough), and I wanted to rename it so that there isn't quite as much confusion between 'ldo->refcount' (ldisk operations refcount) and 'ld->refcount' (ldisc refcount itself) in the same file. So it's now an atomic 'ld->users' count. It still starts at zero, despite having a reference from 'tty->ldisc', but that will change once we turn it into a _real_ refcount. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Tested-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by> Acked-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2009-07-16tty: fix close/hangup raceAlan Cox
We can get a situation where a hangup occurs during or after a close. In that case the ldisc gets disposed of by the close and the hangup then explodes. Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-07-12headers: smp_lock.h reduxAlexey Dobriyan
* Remove smp_lock.h from files which don't need it (including some headers!) * Add smp_lock.h to files which do need it * Make smp_lock.h include conditional in hardirq.h It's needed only for one kernel_locked() usage which is under CONFIG_PREEMPT This will make hardirq.h inclusion cheaper for every PREEMPT=n config (which includes allmodconfig/allyesconfig, BTW) Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-29tty: Fix the leak in tty_ldisc_releaseAlan Cox
Currently we reinit the ldisc on final tty close which is what the old code did to ensure that if the device retained its termios settings then it had the right ldisc. tty_ldisc_reinit does that but also leaves us with the reset ldisc reference which is then leaked. At this point we know the port will be recycled so we can kill the ldisc off completely rather than try and add another ldisc free up when the kref count hits zero. At this point it is safe to keep the ldisc closed as tty_ldisc waiting methods are only used from the user side, and as the final close we are the last such reference. Interrupt/driver side methods will always use the non wait version and get back a NULL. Found with kmemleak and investigated/identified by Catalin Marinas. Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-16ldisc: debug aidsAlan Cox
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-16ldisc: Make sure the ldisc isn't active when we close itAlan Cox
Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-16tty: Fix leaks introduced by the shift to separate ldisc objectsAlan Cox
Gold star for the kmemleak detector. Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-11tty: bring ldisc into CodingStyleAlan Cox
Signed-off-by: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-11tty: Move ldisc_flushAlan Cox
We have a tty_ldisc file now so put tty_ldisc_flush in the right place Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-11tty: rewrite the ldisc lockingAlan Cox
There are several pretty much unfixable races in the old ldisc code, especially with respect to pty behaviour and also to hangup. It's easier to rewrite the code than simply try and patch it up. This patch - splits the ldisc from the tty (so we will be able to refcount it more cleanly later) - introduces a mutex lock for ldisc changing on an active device - fixes the complete mess that hangup caused - implements hopefully correct setldisc/close/hangup locking There are still some problems around pty pairs that have always been there but at least it is now possible to understand the code and fix further problems. This fixes the following known bugs - hang up can leak ldisc references - hang up may not call open/close on ldisc in a matched way - pty/tty pairs can deadlock during an ldisc change - reading the ldisc proc files can cause every ldisc to be loaded and probably a few other of the mysterious ldisc race reports. I'm sure it also adds the odd new one. Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-06-11tty: Extract various bits of ldisc codeAlan Cox
Before trying to tackle the ldisc bugs the code needs to be a good deal more readable, so do the simple extractions of routines first. Signed-off-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-03-31Trim includes of fdtable.hAl Viro
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2009-01-02tty: Fix PPP hang under loadAlan Cox
Signed-off-by: Alan Cox <alan@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2008-08-01try harder to load tty ldisc driverEugeniy Meshcheryakov
Currently function tty_ldisc_get() tries to load an ldisc driver module only when tty_ldisc_try_get() returns -EAGAIN. This happens only if module is being unloaded. If ldisc module is not loaded tty_ldisc_try_get() returns -EINVAL and this case is not handled in tty_ldisc_get(), so request_module() is not called. Attached patch fixes this by calling request_module() if tty_ldisc_try_get() returned any error code. I discovered this when my UMTS modem stopped working with 2.6.27-rc1 because module ppp_async was not loaded. Signed-off-by: Eugeniy Meshcheryakov <eugen@debian.org> Signed-off-by: Alan Cox <alan@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2008-07-22tty: Split ldisc code into its own fileAlan Cox
Signed-off-by: Alan Cox <alan@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>