Commit e9036d0662 ("tty: Drop krefs for interrupted tty lock")
fixed a tty reference counting problem introduced in
commit 0bfd464d3f ("tty: Wait interruptibly for tty lock on reopen"),
so v4.5.0 is correct.
However, commit d6203d0c7b ("tty: Refactor tty_open()") moved the
relevant code for 4.6-rc1; correct the merge.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
User-Mode Linux supplies an alternate TTY_MAJOR driver for stdio console,
so the noctty check in tty_open() must apply only to VT driver tty0
devnode and not the UML console driver tty0 devnode.
Fixes: 11e1d4aa4d ("tty: Consolidate noctty checks in tty_open()")
Reported-by: Richard Weinberger <richard.weinberger@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A small race window exists which allows signal-driven async i/o to be
enabled for the tty when the file ptr has already been hungup and
signal-driven i/o has been disabled:
CPU 0 CPU 1
----- ------
ioctl_fioasync(on)
filp->f_op->fasync(on) __tty_hangup()
tty_fasync(on) tty_lock()
tty_lock() ...
. filp->f_op = &hung_up_tty_fops;
(waiting) __tty_fasync(off)
. tty_unlock()
/* gets tty lock */
/* enables FASYNC */
Check the tty has not been hungup while holding tty_lock.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
VFS uses a two-stage check-and-call method for invoking file_operations
methods, without explicitly snapshotting either the file_operations ptr
or the function ptr. Since the tty core is one of the few VFS users that
changes the f_op file_operations ptr of the file descriptor (when the
tty has been hung up), and since the likelihood of the compiler generating
a reload of either f_op or the function ptr is basically nil, just define
a hung up fasync() file operation that returns an error.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Only the N_TTY line discipline implements the signal-driven i/o
notification enabled/disabled by fcntl(F_SETFL, O_ASYNC). The ldisc
fasync() notification is sent to the ldisc when the enable state has
changed (the tty core is notified via the fasync() VFS file operation).
The N_TTY line discipline used the enable state to change the wakeup
condition (minimum_to_wake = 1) for notifying the signal handler i/o is
available. However, just the presence of data is sufficient and necessary
to signal i/o is available, so changing minimum_to_wake is unnecessary
(and creates a race condition with read() and poll() which may be
concurrently updating minimum_to_wake).
Furthermore, since the kill_fasync() VFS helper performs no action if
the fasync list is empty, calling unconditionally is preferred; if
signal driven i/o just has been disabled, no signal will be sent by
kill_fasync() anyway so notification of the change via the ldisc
fasync() method is superfluous.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Access to tty->tty_files list is always per-tty, never for all ttys
simultaneously. Replace global tty_files_lock spinlock with per-tty
->files_lock. Initialize when the ->tty_files list is inited, in
alloc_tty_struct().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Move is_ignored() to drivers/tty/tty_io.c and re-declare in file
scope.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reduce global tty symbols; move and rename tty_ldisc_begin() as
n_tty_init() and redefine the N_TTY ldisc ops as file scope.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_mutex is a core, system-wide lock; there is no reason for any
code outside the tty core to have direct access.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The line discipline id is stored in the tty's termios; document the
implicit initial value of N_TTY.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Currently, when the tty is hungup, the ldisc is re-instanced; ie., the
current instance is destroyed and a new instance is created. The purpose
of this design was to guarantee a valid, open ldisc for the lifetime of
the tty.
However, now that tty buffers are owned by and have lifetime equivalent
to the tty_port (since v3.10), any data received immediately after the
ldisc is re-instanced may cause continued driver i/o operations
concurrently with the driver's hangup() operation. For drivers that
shutdown h/w on hangup, this is unexpected and usually bad. For example,
the serial core may free the xmit buffer page concurrently with an
in-progress write() operation (triggered by echo).
With the existing stable and robust ldisc reference handling, the
cleaned-up tty_reopen(), the straggling unsafe ldisc use cleaned up, and
the preparation to properly handle a NULL tty->ldisc, the ldisc instance
can be destroyed and only re-instanced when the tty is re-opened.
If the tty was opened as /dev/console or /dev/tty0, the original behavior
of re-instancing the ldisc is retained (the 'reinit' parameter to
tty_ldisc_hangup() is true). This is required since those file descriptors
are never hungup.
This patch has neglible impact on userspace; the tty file_operations ptr
is changed to point to the hungup file operations _before_ the ldisc
instance is destroyed, so only racing file operations might now retrieve
a NULL ldisc reference (which is simply handled as if the hungup file
operation had been called instead -- see "tty: Prepare for destroying
line discipline on hangup").
This resolves a long-standing FIXME and several crash reports.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty->ldisc is a ptr to struct tty_ldisc, but unfortunately 'ldisc' is
also used as a parameter or local name to refer to the line discipline
index value (ie, N_TTY, N_GSM, etc.); instead prefer the name used
by the line discipline registration/ref counting functions.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty file_operations (read/write/ioctl) wait for the ldisc reference
indefinitely (until ldisc lifetime events, such as hangup or TIOCSETD,
finish). Since hangup now destroys the ldisc and does not instance
another copy, file_operations must now be prepared to receive a NULL
ldisc reference from tty_ldisc_ref_wait():
CPU 0 CPU 1
----- -----
(*f_op->read)() => tty_read()
__tty_hangup()
...
f_op = &hung_up_tty_fops;
...
tty_ldisc_hangup()
tty_ldisc_lock()
tty_ldisc_kill()
tty->ldisc = NULL
tty_ldisc_unlock()
ld = tty_ldisc_ref_wait()
/* ld == NULL */
Instead, the action taken now is to return the same value as if the
tty had been hungup a moment earlier:
CPU 0 CPU 1
----- -----
__tty_hangup()
...
f_op = &hung_up_tty_fops;
(*f_op->read)() => hung_up_tty_read()
return 0;
...
tty_ldisc_hangup()
tty_ldisc_lock()
tty_ldisc_kill()
tty->ldisc = NULL
tty_ldisc_unlock()
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
After the ldisc is released, but before the tty is destroyed, the termios
is saved (in tty_free_termios()); this termios is restored if a new
tty is created on next open(). However, the line discipline is always
reset, which is not obvious in the current method. Instead, reset
as part of the restore.
Restore the original line discipline, which may not have been N_TTY.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Extract the driver lookup and reopen-or-initialize logic into helper
function tty_open_by_driver(). No functional change.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Evaluate the conditions which prevent this tty being the controlling
terminal in one place, just before setting the controlling terminal.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_driver_remove_tty() is only local-scope; declare as static.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_init_termios() never returns an error; re-declare as void. Remove
unnecessary error handling from callers. Remove extern declarations
of tty_free_termios() and free_tty_struct() and re-declare in file
scope.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
free_tty_struct() is never called with NULL tty; the two call sites
would already have faulted on earlier access.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
release_tty() leaks the ldisc instance when called directly (rather
than when releasing the file descriptor from tty_release()).
Since tty_ldisc_release() clears tty->ldisc, releasing the ldisc
instance at tty teardown if tty->ldisc is non-null is not in danger
of double-releasing the ldisc.
Remove deinitialize_tty_struct() now that free_tty_struct() always
performs the tty_ldisc_deinit().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
ioctl(TIOCGETD) retrieves the line discipline id directly from the
ldisc because the line discipline id (c_line) in termios is untrustworthy;
userspace may have set termios via ioctl(TCSETS*) without actually
changing the line discipline via ioctl(TIOCSETD).
However, directly accessing the current ldisc via tty->ldisc is
unsafe; the ldisc ptr dereferenced may be stale if the line discipline
is changing via ioctl(TIOCSETD) or hangup.
Wait for the line discipline reference (just like read() or write())
to retrieve the "current" line discipline id.
Cc: <stable@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A small window exists where a tty reopen will observe the tty
just prior to imminent teardown (tty->count == 0); in this case, open()
returns EIO to userspace.
Instead, retry the open after checking for signals and yielding;
this interruptible retry loop allows teardown to commence and initialize
a new tty on retry. Never retry the BSD master pty reopen; there is no
guarantee the pty pair teardown is imminent since the slave file
descriptors may remain open indefinitely.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Cc: stable <stable@vger.kernel.org> # 4.4
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Allow a signal to interrupt the wait for a tty reopen; eg., if
the tty has starting final close and is waiting for the device to
drain.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Cc: stable <stable@vger.kernel.org> # 4.4
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A master pty should never be a controlling tty in Linux; if the
master pty is specified to ioctl(TIOCSCTTY), silently substitute the slave
pty as the controlling tty.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Where possible, use dev_dbg() instead of pr_debug()
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that tty_debug() macro uses pr_debug(), the function name can
be printed when using dynamic debug; printing the function name within
the format string is redundant.
Remove the __func__ parameter and print specifier from the format string.
Add context to messages for when the function name is not printed by
dynamic debug, or when dynamic debug is not enabled.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Convert remaining printk() use to pr_*() when tty is unknown or
unsafe to use.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Include the driver name in the tty_register_device_attr() error
message for invalid index.
Note that tty_err() cannot be used here because there is no tty;
use pr_err().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use tty_notice() for unified message format from the tty core.
Fix each message to accurately reflect the cause of each termination.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since not all ttys are devices (eg., SysV ptys), dev_*() printk macros
cannot be used. Define tty_*() printk macros that output in similar
format to dev_*() macros (ie., <driver> <tty>: .....).
Transform the most-trivial printk( LEVEL ...) usage to tty_*() usage.
NB: The function name has been eliminated from messages with unique
context, or prefixed to the format when given.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_paranoia_check() is only used within drivers/tty/tty_io.c;
remove extern declaration in header and limit symbol to file scope.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The correct lock order is atomic_write_lock => termios_rwsem, as
established by tty_write() => n_tty_write().
Fixes: c274f6ef1c ("tty: Hold termios_rwsem for tcflow(TCIxxx)")
Reported-and-Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org> # v3.18+
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Introduce API functions to restart and cancel tty buffer work, rather
than manipulate buffer work directly.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_write_message() allows the caller to directly write to a specific
tty. Since the line discipline is bypassed for the direct write,
nothing prevents the tty from being torn down after the tty count is
checked.
Hold the tty lock for the duration of the direct write.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tiocspgrp() is the ioctl handler for TIOCSPGRP, which runs in
non-atomic context; use spin_lock/unlock_irq (since interrupt state
is on).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The job_control() check in n_tty_read() has nearly identical purpose
and results as tty_check_change(). Both functions' purpose is to
determine if the current task's pgrp is the foreground pgrp for the tty,
and if not, to signal the current pgrp.
Introduce __tty_check_change() which takes the signal to send
and performs the shared operations for job control() and
tty_check_change().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This is mostly a hardening fix, given that write-only access to other
users' ttys is usually only given through setgid tty executables.
Signed-off-by: Jann Horn <jann@thejh.net>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit a3a10ce342 ("Avoid usb reset crashes by making tty_io cdevs truly
dynamic") which mixes using cdev_alloc() and cdev_init() is problematic.
Subsequent call to cdev_init() after cdev_alloc() sets kobj release method
from cdev_dynamic_release() to cdev_default_release() and thus makes it
impossible to free allocated cdev.
This patch also consolidates error path of cdev_add() as cdev can also leak
here if things went wrong.
Signed-off-by: Leon Yu <chianglungyu@gmail.com>
Fixes: a3a10ce342 ("Avoid usb reset crashes by making tty_io cdevs truly dynamic")
Acked-by: Richard Watts <rrw@kynesim.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Introduce tty_debug() macro to output uniform debug information for
tty core debug messages (function name and tty name).
Note: printk(KERN_DEBUG) is retained here over pr_debug() since
messages can be enabled in non-DEBUG builds.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Output the function name, tty name, and invariant failure (if applicable).
Add the tty count to the tty_open() message. Fix the disassociate_ctty()
message, which printed the NULL pointer and the wrong message.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
task_pgrp requires an rcu or tasklist lock to be obtained if the returned pid
is to be dereferenced, which kill_pgrp does. Obtain an RCU lock for the
duration of use.
Signed-off-by: Patrick Donnelly <batrick@batbytes.com>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_name no longer uses the buf parameter, so remove it along with all
the 64 byte stack buffers that used to be passed in.
Mostly generated by the coccinelle script
@depends on patch@
identifier buf;
constant C;
expression tty;
@@
- char buf[C];
<+...
- tty_name(tty, buf)
+ tty_name(tty)
...+>
allmodconfig compiles, so I'm fairly confident the stack buffers
weren't used for other purposes as well.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
All users of tty_name pass the return value (the provided buffer) to
some printf-like function. We can thus avoid the strcpy and, more
importantly, later remove the buf parameter completely, eliminating
the need for some 64 byte stack buffers.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
All users of tty_name pass the result directly to a printf-like
function. This means we can actually let tty_name return the literal
"NULL tty" or tty->name directly, avoiding the strcpy and a lot of
medium-sized stack buffers. In preparation for that, make the return
type const char*.
While at it, we can also constify the tty parameter.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We only care if anything other than the lower 3 bits of the tty has
changed, so just check that way, which makes it a bit faster, and more
obvious what is going on. Also, document this for future developers to
understand why we did this.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
Instead of manual calls of device_create_file() and
device_remove_file(), pass the static attribute groups using
device_create_with_groups().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This problem was taken care of three times already in
* b0de59b573 (TTY: do not update
atime/mtime on read/write),
* 37b7f3c765 (TTY: fix atime/mtime
regression), and
* b0b885657b (tty: fix up atime/mtime
mess, take three)
But it still misses one point. As John Paul correctly points out, we
do not care about setting date. If somebody ever changes wall
time backwards (by mistake for example), tty timestamps are never
updated until the original wall time passes.
So check the absolute difference of times and if it large than "8
seconds or so", always update the time. That means we will update
immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
check, but it was always that way.
Thanks John for serving me this so nicely debugged.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: John Paul Perry <john_paul.perry@alcatel-lucent.com>
Cc: <stable@vger.kernel.org> # all, as b0b885657 was backported
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Exclusive mode ttys (TTY_EXCLUSIVE) do not allow further reopens;
fail the condition before associating the file pointer and calling
the driver open() method.
Prevents DTR programming when the tty is already in exclusive mode.
Reported-by: Shreyas Bethur <shreyas.bethur@ni.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Shreyas Bethur <shreyas.bethur@ni.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The functions put_device() and tty_kref_put() test whether their argument
is NULL and then return immediately.
Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When somebody calls TIOCSSERIAL ioctl with serial flags to set one of
* ASYNC_SESSION_LOCKOUT
* ASYNC_PGRP_LOCKOUT
* ASYNC_CALLOUT_NOHUP
* ASYNC_AUTOPROBE
nothing happens. We actually ignore the flags for over a decade at
least (I checked 2.6.0).
So start yelling at users who use those flags, that they shouldn't.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The use of older function ptr calling style, (*fn)(), makes static
analysis more error-prone; replace with modern fn() style.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_hung_up_p() is equivalent to the open-coded test in tty_open().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We need the fixes in drivers/tty/tty_io.c that were done in there for
future patches in this branch.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_ldisc_flush() first clears the line discipline input buffer,
then clears the tty flip buffers. However, this allows for existing
data in the tty flip buffers to be added after the ldisc input
buffer has been cleared, but before the flip buffers have been cleared.
Add an optional ldisc parameter to tty_buffer_flush() to allow
tty_ldisc_flush() to pass the ldisc to clear.
NB: Initially, the plan was to do this automatically in
tty_buffer_flush(). However, an audit of the behavior of existing
line disciplines showed that performing a ldisc buffer flush on
ioctl(TCFLSH) was not always the outcome. For example, some line
disciplines have flush_buffer() methods but not ioctl() methods,
so a ->flush_buffer() command would be unexpected.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When releasing the master pty, the slave pty also needs to be locked
to prevent concurrent tty count changes for the slave pty and to
ensure that only one parallel master and slave release observe the
final close, and proceed to destruct the pty pair. Conversely, when
releasing the slave pty, locking the master pty is not necessary
(since the master's state can be inferred by the slave tty count).
Introduce tty_lock_slave()/tty_unlock_slave() which acquires/releases
the tty lock of the slave pty. Remove tty_lock_pair()/tty_unlock_pair().
Dropping the tty_lock is no longer required to re-establish a stable
lock order.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The local o_tty variable in tty_release() is now accessed only
when closing the pty master.
Set o_tty to slave pty when closing pty master, otherwise NULL;
use o_tty != NULL as replacement for pty_master.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Passing the 'other' tty to tty_release_checks() only makes sense
for a pty pair; make o_tty scope local instead.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Passing the 'other' tty to tty_ldisc_release() only makes sense
for a pty pair; make o_tty function local instead.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Perform work flush for both ends of a pty pair within tty_flush_works(),
rather than calling twice.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When the slave side closes and its tty count is 0, the pty
pair can be destroyed; the master side must have already
closed for the slave side tty count to be 0. Thus, only the
pty master close must check if the slave side has closed by
checking the slave tty count.
Remove the pre-computed closing flags and check the actual count(s).
Regular ttys are unaffected by this change.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Holding the tty_lock() is necessary to prevent concurrent changes
to the tty count that may cause it to differ from the open file
list count. The tty_lock() is already held at all call sites.
NB: Note that the check for the pty master tty count is safe because
the slave's tty_lock() is held while decrementing the pty master
tty count.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Releasing the tty locks while waiting for the tty wait queues to
be empty is no longer necessary nor desirable. Prior to
"tty: Don't take tty_mutex for tty count changes", dropping the
tty locks was necessary to reestablish the correct lock order between
tty_mutex and the tty locks. Dropping the global tty_mutex was necessary;
otherwise new ttys could not have been opened while waiting.
However, without needing the global tty_mutex held, the tty locks for
the releasing tty can now be held through the sleep. The sanity check
is for abnormal conditions caused by kernel bugs, not for recoverable
errors caused by misbehaving userspace; dropping the tty locks only
allows the tty state to get more sideways.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Holding tty_mutex is no longer required to serialize changes to
the tty_count or to prevent concurrent opens of closing ttys;
tty_lock() is sufficient.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that re-open is not permitted for a legacy BSD pty master,
using TTY_CLOSING to indicate when a tty can be torn-down is
no longer necessary.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Holding tty_mutex for a tty re-open is no longer necessary since
"tty: Clarify re-open behavior of master ptys". Because the
slave tty count is no longer accessed by tty_reopen(), holding
tty_mutex to prevent concurrent final tty_release() of the slave
pty is not required.
As with "tty: Re-open /dev/tty without tty_mutex", holding a
tty kref until the tty_lock is acquired is sufficient to ensure
the tty has not been freed, which, in turn, is sufficient to
ensure the tty_lock can be safely acquired and the tty count
can be safely retrieved. A non-zero tty count with the tty lock
held guarantees that release_tty() has not run and cannot
run concurrently with tty_reopen().
Change tty_driver_lookup_tty() to acquire the tty kref, which
allows the tty_mutex to be dropped before acquiring the tty lock.
Dropping the tty_mutex before attempting the tty_lock allows
other ttys to be opened and released, without needing this
tty_reopen() to complete.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Opening /dev/tty (ie., the controlling tty for the current task)
is always a re-open of the underlying tty. Because holding the
tty_lock is sufficient for safely re-opening a tty, and because
having a tty kref is sufficient for safely acquiring the tty_lock [1],
tty_open_current_tty() does not require holding tty_mutex.
Repurpose tty_open_current_tty() to perform the re-open itself and
refactor tty_open().
[1] Analysis of safely re-opening the current tty w/o tty_mutex
get_current_tty() gets a tty kref from the already kref'ed tty value of
current->signal->tty while holding the sighand lock for the current
task. This guarantees that the tty pointer returned from
get_current_tty() points to a tty which remains referenceable
while holding the kref.
Although release_tty() may run concurrently, and thus the driver
reference may be removed, release_one_tty() cannot have run, and
won't while holding the tty kref.
This, in turn, guarantees the tty_lock() can safely be acquired
(since tty->magic and tty->legacy_mutex are still a valid dereferences).
The tty_lock() also gets a tty kref to prevent the tty_unlock() from
dereferencing a released tty. Thus, the kref returned from
get_current_tty() can be released.
Lastly, the first operation of tty_reopen() is to check the tty count.
If non-zero, this ensures release_tty() is not running concurrently,
and the driver references have not been removed.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Although perhaps not obvious, the TTY_CLOSING bit is set when the
tty count has been decremented to 0 (which occurs while holding
tty_lock). The only other case when tty count is 0 during a re-open
is when a legacy BSD pty master has been opened in parallel but
after the pty slave, which is unsupported and returns an error.
Thus !tty->count contains the complete set of degenerate conditions
under which a tty open fails.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Re-opening master ptys is not allowed. Once opened and for the remaining
lifetime of the master pty, its tty count is 1. If its tty count has
dropped to 0, then the master pty was closed and TTY_CLOSING was set,
and destruction may begin imminently.
Besides the normal case of a legacy BSD pty master being re-opened
(which always returns -EIO), this code is only reachable in 2 degenerate
cases:
1. The pty master is the controlling terminal (this is possible through
the TIOCSCTTY ioctl). pty masters are not designed to be controlling
terminals and it's an oversight that tiocsctty() ever let that happen.
The attempted open of /dev/tty will always fail. No known program does
this.
2. The legacy BSD pty slave was opened first. The slave open will fail
in pty_open() and tty_release() will commence. But before tty_release()
claims the tty_mutex, there is a very small window where a parallel
master open might succeed. In a test of racing legacy BSD slave and
master parallel opens, where:
slave open attempts: 10000 success:4527 failure:5473
master open attempts: 11728 success:5789 failure:5939
only 8 master open attempts would have succeeded reaching this code and
successfully opened the master pty. This case is not possible with
SysV ptys.
Always return -EIO if a master pty is re-opened or the slave is opened
first and the master opened in parallel (for legacy BSD ptys).
Furthermore, now that changing the slave's count is not required,
the tty_lock is sufficient for preventing concurrent changes to the
tty being re-opened (or failing re-opening).
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that tty_ldisc_hangup() does not drop the tty lock, it is no
longer possible to observe TTY_HUPPING while holding the tty lock
on another cpu.
Remove TTY_HUPPING bit definition.
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The session and foreground process group pid references will be
non-NULL if tiocsctty() is stealing the controlling tty from another
session (ie., arg == 1 in tiocsctty()).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Setting the controlling terminal for a session occurs with either
the first open of a non-pty master tty or with ioctl(TIOCSCTTY).
Since only the session leader can set the controlling terminal for
a session (and the session leader cannot change), it is not
necessary to prevent a process from attempting to set different
ttys as the controlling terminal concurrently.
So it's only necessary to prevent the same tty from becoming the
controlling terminal for different session leaders. The tty_lock()
is sufficient to prevent concurrent proc_set_tty() for the same
tty.
Remove the tty_mutex lock region; add tty_lock() to tiocsctty().
While this may appear to allow a race condition between opening
the controlling tty via tty_open_current_tty() and stealing the
controlling tty via ioctl(TIOCSCTTY, 1), that race condition already
existed. Even if the tty_mutex prevented stealing the controlling tty
while tty_open_current_tty() returned the original controlling tty,
it cannot prevent stealing the controlling tty before tty_open() returns.
Thus, tty_open() could already return a no-longer-controlling tty when
opening /dev/tty.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tiocspgrp() is the lone caller of session_of_pgrp(); relocate and
limit to file scope.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Claim a read lock on the tasklist_lock while setting the controlling
terminal for the session leader. This fixes multiple races:
1. task_pgrp() and task_session() cannot be safely dereferenced, such
as passing to get_pid(), without holding either rcu_read_lock() or
tasklist_lock
2. setsid() unwisely allows any thread in the thread group to
make the thread group leader the session leader; this makes the
unlocked reads of ->signal->leader and signal->tty potentially
unordered, stale or even have spurious values.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The tty parameter to __proc_set_tty() cannot be NULL; all
call sites have already dereferenced tty.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Only the current task itself can set its controlling tty (other
than before the task has been forked). Equivalent to existing usage.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Move the controlling tty-related functions and remove forward
declarations for __proc_set_tty() and proc_set_tty().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty_pair_get_pty() has no in-tree users and tty_pair_get_tty()
has only one file-local user. Remove the external declarations,
the export declarations, and declare tty_pair_get_tty() static.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
sparse lock annotations cannot represent conditional acquire, such
as mutex_lock_interruptible() or mutex_trylock(), and produce sparse
warnings at _every_ correct call site.
Remove lock annotations from tty_write_lock() and tty_write_unlock().
Fixes sparse warnings:
drivers/tty/tty_io.c:1083:13: warning: context imbalance in 'tty_write_unlock' - wrong count at exit
drivers/tty/tty_io.c:1090:12: warning: context imbalance in 'tty_write_lock' - wrong count at exit
drivers/tty/tty_io.c:1211:17: warning: context imbalance in 'tty_write_message' - unexpected unlock
drivers/tty/tty_io.c:1233:16: warning: context imbalance in 'tty_write' - different lock contexts for basic block
drivers/tty/tty_io.c:1285:5: warning: context imbalance in 'tty_send_xchar' - different lock contexts for basic block
drivers/tty/tty_io.c:2653:12: warning: context imbalance in 'send_break' - different lock contexts for basic block
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Only print one warning when a task is on the read_wait or write_wait
wait queue at final tty release.
Cc: <stable@vger.kernel.org> # 3.4.x+
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Kernel oops can cause the tty to be unreleaseable (for example, if
n_tty_read() crashes while on the read_wait queue). This will cause
tty_release() to endlessly loop without sleeping.
Use a killable sleep timeout which grows by 2n+1 jiffies over the interval
[0, 120 secs.) and then jumps to forever (but still killable).
NB: killable just allows for the task to be rewoken manually, not
to be terminated.
Cc: <stable@vger.kernel.org> # since before 2.6.32
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAABAgAGBQJUNZK4AAoJEAAOaEEZVoIVI08P/iM7eaIVRnqaqtWw/JBzxiba
EMDlJYUBSlv6lYk9s8RJT4bMmcmGAKSYzVAHSoPahzNcqTDdFLeDTLGxJ8uKBbjf
d1qRRdH1yZHGUzCvJq3mEendjfXn435Y3YburUxjLfmzrzW7EbMvndiQsS5dhAm9
PEZ+wrKF/zFL7LuXa1YznYrbqOD/GRsJAXGEWc3kNwfS9avephVG/RI3GtpI2PJj
RY1mf8P7+WOlrShYoEuUo5aqs01MnU70LbqGHzY8/QKH+Cb0SOkCHZPZyClpiA+G
MMJ+o2XWcif3BZYz+dobwz/FpNZ0Bar102xvm2E8fqByr/T20JFjzooTKsQ+PtCk
DetQptrU2gtyZDKtInJUQSDPrs4cvA13TW+OEB1tT8rKBnmyEbY3/TxBpBTB9E6j
eb/V3iuWnywR3iE+yyvx24Qe7Pov6deM31s46+Vj+GQDuWmAUJXemhfzPtZiYpMT
exMXTyDS3j+W+kKqHblfU5f+Bh1eYGpG2m43wJVMLXKV7NwDf8nVV+Wea962ga+w
BAM3ia4JRVgRWJBPsnre3lvGT5kKPyfTZsoG+kOfRxiorus2OABoK+SIZBZ+c65V
Xh8VH5p3qyCUBOynXlHJWFqYWe2wH0LfbPrwe9dQwTwON51WF082EMG5zxTG0Ymf
J2z9Shz68zu0ok8cuSlo
=Hhee
-----END PGP SIGNATURE-----
Merge tag 'locks-v3.18-1' of git://git.samba.org/jlayton/linux
Pull file locking related changes from Jeff Layton:
"This release is a little more busy for file locking changes than the
last:
- a set of patches from Kinglong Mee to fix the lockowner handling in
knfsd
- a pile of cleanups to the internal file lease API. This should get
us a bit closer to allowing for setlease methods that can block.
There are some dependencies between mine and Bruce's trees this cycle,
and I based my tree on top of the requisite patches in Bruce's tree"
* tag 'locks-v3.18-1' of git://git.samba.org/jlayton/linux: (26 commits)
locks: fix fcntl_setlease/getlease return when !CONFIG_FILE_LOCKING
locks: flock_make_lock should return a struct file_lock (or PTR_ERR)
locks: set fl_owner for leases to filp instead of current->files
locks: give lm_break a return value
locks: __break_lease cleanup in preparation of allowing direct removal of leases
locks: remove i_have_this_lease check from __break_lease
locks: move freeing of leases outside of i_lock
locks: move i_lock acquisition into generic_*_lease handlers
locks: define a lm_setup handler for leases
locks: plumb a "priv" pointer into the setlease routines
nfsd: don't keep a pointer to the lease in nfs4_file
locks: clean up vfs_setlease kerneldoc comments
locks: generic_delete_lease doesn't need a file_lock at all
nfsd: fix potential lease memory leak in nfs4_setlease
locks: close potential race in lease_get_mtime
security: make security_file_set_fowner, f_setown and __f_setown void return
locks: consolidate "nolease" routines
locks: remove lock_may_read and lock_may_write
lockd: rip out deferred lock handling from testlock codepath
NFSD: Get reference of lockowner when coping file_lock
...
Relocate the file-scope function, send_prio_char(), as a global
helper tty_send_xchar(). Remove the global declarations for
tty_write_lock()/tty_write_unlock(), as these are file-scope only now.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When a master pty is set to packet mode, flow control changes to
the slave pty cause notifications to the master pty via reads and
polls. However, these tests are occurring for all ttys, not
just ptys.
Implement flow control packet mode notifications in the pty driver.
Only the slave side implements the flow control handlers since
packet mode is asymmetric; the master pty receives notifications
for slave-side changes, but not vice versa.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Without serialization, the flow control state can become inverted
wrt. the actual hardware state. For example,
CPU 0 | CPU 1
stop_tty() |
lock ctrl_lock |
tty->stopped = 1 |
unlock ctrl_lock |
| start_tty()
| lock ctrl_lock
| tty->stopped = 0
| unlock ctrl_lock
| driver->start()
driver->stop() |
In this case, the flow control state now indicates the tty has
been started, but the actual hardware state has actually been stopped.
Introduce tty->flow_lock spinlock to serialize tty flow control changes.
Split out unlocked __start_tty()/__stop_tty() flavors for use by
ioctl(TCXONC) in follow-on patch.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
security_file_set_fowner always returns 0, so make it f_setown and
__f_setown void return functions and fix up the error handling in the
callers.
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
The two functions alloc_tty_struct and initialize_tty_struct are
always called together. Merge them into alloc_tty_struct, updating its
prototype and the only two callers of these functions.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The 'active' sysfs attribute should refer to the currently active tty
devices the console is running on, not the currently active console. The
console structure doesn't refer to any device in sysfs, only the tty the
console is running on has. So we need to print out the tty names in
'active', not the console names.
There is one special-case, which is tty0. If the console is directed to
it, we want 'tty0' to show up in the file, so user-space knows that the
messages get forwarded to the active VT. The ->device() callback would
resolve tty0, though. Hence, treat it special and don't call into the VT
layer to resolve it (plymouth is known to depend on it).
Cc: Lennart Poettering <lennart@poettering.net>
Cc: Kay Sievers <kay@vrfy.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Werner Fink <werner@suse.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This reverts commit d8a5dc3033.
This breaks plymouth installs, either because plymouth is using the file
"incorrectly" or because the patch is incorrect. Either way, this needs
to be reverted until it is all figured out.
Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
Reported-by: Ray Strode <halfline@gmail.com>
Cc: Lennart Poettering <lennart@poettering.net>
Cc: Kay Sievers <kay@vrfy.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Werner Fink <werner@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The 'active' sysfs attribute should refer to the currently active tty
devices the console is running on, not the currently active console.
The console structure doesn't refer to any device in sysfs, only the tty
the console is running on has. So we need to print out the tty names in
'active', not the console names.
This resolves an issue on s390 platforms in determining the correct
console device to use.
Cc: Lennart Poettering <lennart@poettering.net>
Cc: Kay Sievers <kay@vrfy.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Werner Fink <werner@suse.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A common security idiom is to hangup the current tty (via vhangup())
after forking but before execing a root shell. This hangs up any
existing opens which other processes may have and ensures subsequent
opens have the necessary permissions to open the root shell tty/pty.
Reset the TTY_HUPPED state after the driver has successfully
returned the opened tty (perform the reset while the tty is locked
to avoid racing with concurrent hangups).
Reported-by: Heorhi Valakhanovich <valahanovich@tut.by>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Cc: stable <stable@vger.kernel.org> # 3.12
Tested-by: Heorhi Valakhanovich <valahanovich@tut.by>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Starting from v3.10 (probably commit f91e259041: "tty: Signal
foreground group processes in hangup") disassociate_ctty() sends SIGCONT
if tty && on_exit. This breaks LSB test-suite, in particular test8 in
_exit.c and test40 in sigcon5.c.
Put the "!on_exit" check back to restore the old behaviour.
Review by Peter Hurley:
"Yes, this regression was introduced by me in that commit. The effect
of the regression is that ptys will receive a SIGCONT when, in similar
circumstances, ttys would not.
The fact that two test vectors accidentally tripped over this
regression suggests that some other apps may as well.
Thanks for catching this"
Cc: stable@vger.kernel.org # v3.10+
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reported-by: Karel Srot <ksrot@redhat.com>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Instrumented testing shows a tty can be hungup multiple times [1].
Although concurrent hangups are properly serialized, multiple
hangups for the same tty should be prevented.
If tty has already been HUPPED, abort hangup. Note it is not
necessary to cleanup file *redirect on subsequent hangups,
as only TIOCCONS can set that value and ioctls are disabled
after hangup.
[1]
Test performed by simulating a concurrent async hangup via
tty_hangup() with a sync hangup via tty_vhangup(), while
__tty_hangup() was instrumented with:
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 26bb78c..fe8b061 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -629,6 +629,8 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
tty_lock(tty);
+ WARN_ON(test_bit(TTY_HUPPED, &tty->flags));
+
/* some functions below drop BTM, so we need this bit */
set_bit(TTY_HUPPING, &tty->flags);
Test result:
WARNING: at /home/peter/src/kernels/mainline/drivers/tty/tty_io.c:632 __tty_hangup+0x459/0x460()
Modules linked in: ip6table_filter ip6_tables ebtable_nat <...snip...>
CPU: 6 PID: 1197 Comm: kworker/6:2 Not tainted 3.10.0-0+rfcomm-xeon #0+rfcomm
Hardware name: Dell Inc. Precision WorkStation T5400 /0RW203, BIOS A11 04/30/2012
Workqueue: events do_tty_hangup
0000000000000009 ffff8802b16d7d18 ffffffff816b553e ffff8802b16d7d58
ffffffff810407e0 ffff880254f95c00 ffff880254f95c00 ffff8802bfd92b00
ffff8802bfd96b00 ffff880254f95e40 0000000000000180 ffff8802b16d7d68
Call Trace:
[<ffffffff816b553e>] dump_stack+0x19/0x1b
[<ffffffff810407e0>] warn_slowpath_common+0x70/0xa0
[<ffffffff8104082a>] warn_slowpath_null+0x1a/0x20
[<ffffffff813fb279>] __tty_hangup+0x459/0x460
[<ffffffff8107409c>] ? finish_task_switch+0xbc/0xe0
[<ffffffff813fb297>] do_tty_hangup+0x17/0x20
[<ffffffff8105fd6f>] process_one_work+0x16f/0x450
[<ffffffff8106007c>] process_scheduled_works+0x2c/0x40
[<ffffffff8106060a>] worker_thread+0x26a/0x380
[<ffffffff810603a0>] ? rescuer_thread+0x310/0x310
[<ffffffff810698a0>] kthread+0xc0/0xd0
[<ffffffff816b0000>] ? destroy_compound_page+0x65/0x92
[<ffffffff810697e0>] ? kthread_create_on_node+0x130/0x130
[<ffffffff816c495c>] ret_from_fork+0x7c/0xb0
[<ffffffff810697e0>] ? kthread_create_on_node+0x130/0x130
---[ end trace 98d9f01536cf411e ]---
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
