If a tty is opened on a serial console, don't drop DTR on
last tty close, on tty hangup, or when resetting port hardware
via TIOCSSERIAL and TIOCSERCONFIG ioctls.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/serial_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index b68550d..498b149 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -239,7 +239,8 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
/*
* Turn off DTR and RTS early.
*/
- if (!tty || (tty->termios.c_cflag & HUPCL))
+ if (!uart_console(uport) &&
+ (!tty || (tty->termios.c_cflag & HUPCL)))
uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
uart_port_shutdown(port);
--
1.9.1
When a tty is opened for the serial console, the termios c_cflag
settings are inherited from the console line settings.
However, if the tty is subsequently closed, the termios settings
are lost. This results in a garbled console if the console is later
suspended and resumed.
Preserve the termios c_cflag for the serial console when the tty
is shutdown; this reflects the most recent line settings.
Fixes: Bugzilla #69751, 'serial console does not wake from S3'
Reported-by: Valerio Vanni <[email protected]>
Cc: Alan Cox <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/serial_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 498b149..a67861c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -239,6 +239,9 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
/*
* Turn off DTR and RTS early.
*/
+ if (uart_console(uport) && tty)
+ uport->cons->cflag = tty->termios.c_cflag;
+
if (!uart_console(uport) &&
(!tty || (tty->termios.c_cflag & HUPCL)))
uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
--
1.9.1
On Wed, Jun 04, 2014 at 10:16:10AM -0400, Peter Hurley wrote:
> If a tty is opened on a serial console, don't drop DTR on
> last tty close, on tty hangup, or when resetting port hardware
> via TIOCSSERIAL and TIOCSERCONFIG ioctls.
>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> drivers/tty/serial/serial_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Is this a regression, or something we have never done?
thanks,
greg k-h
On Wed, Jun 04, 2014 at 10:16:11AM -0400, Peter Hurley wrote:
> When a tty is opened for the serial console, the termios c_cflag
> settings are inherited from the console line settings.
> However, if the tty is subsequently closed, the termios settings
> are lost. This results in a garbled console if the console is later
> suspended and resumed.
>
> Preserve the termios c_cflag for the serial console when the tty
> is shutdown; this reflects the most recent line settings.
>
> Fixes: Bugzilla #69751, 'serial console does not wake from S3'
> Reported-by: Valerio Vanni <[email protected]>
> Cc: Alan Cox <[email protected]>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> drivers/tty/serial/serial_core.c | 3 +++
> 1 file changed, 3 insertions(+)
Same as before, regression, or just a normal "new feature"?
greg k-h
On 06/04/2014 10:22 AM, Greg Kroah-Hartman wrote:
> On Wed, Jun 04, 2014 at 10:16:10AM -0400, Peter Hurley wrote:
>> If a tty is opened on a serial console, don't drop DTR on
>> last tty close, on tty hangup, or when resetting port hardware
>> via TIOCSSERIAL and TIOCSERCONFIG ioctls.
>>
>> Signed-off-by: Peter Hurley <[email protected]>
>> ---
>> drivers/tty/serial/serial_core.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Is this a regression, or something we have never done?
Not sure. I just noticed this was wrong while working on the
other fix.
Regards,
Peter Hurley
On 06/04/2014 10:22 AM, Greg Kroah-Hartman wrote:
> On Wed, Jun 04, 2014 at 10:16:11AM -0400, Peter Hurley wrote:
>> When a tty is opened for the serial console, the termios c_cflag
>> settings are inherited from the console line settings.
>> However, if the tty is subsequently closed, the termios settings
>> are lost. This results in a garbled console if the console is later
>> suspended and resumed.
>>
>> Preserve the termios c_cflag for the serial console when the tty
>> is shutdown; this reflects the most recent line settings.
>>
>> Fixes: Bugzilla #69751, 'serial console does not wake from S3'
>> Reported-by: Valerio Vanni <[email protected]>
>> Cc: Alan Cox <[email protected]>
>> Signed-off-by: Peter Hurley <[email protected]>
>> ---
>> drivers/tty/serial/serial_core.c | 3 +++
>> 1 file changed, 3 insertions(+)
>
> Same as before, regression, or just a normal "new feature"?
This was reported as a regression since 2.6.24, but that was likely
misreported, and more likely due to a userspace update which triggers
the bug.
I would be surprised if console resume _never_ worked if the serial tty
was opened then closed, but it probably has worked for a long time.
Regards,
Peter Hurley
On 06/04/2014 10:46 AM, Peter Hurley wrote:
> On 06/04/2014 10:22 AM, Greg Kroah-Hartman wrote:
>> On Wed, Jun 04, 2014 at 10:16:11AM -0400, Peter Hurley wrote:
>>> When a tty is opened for the serial console, the termios c_cflag
>>> settings are inherited from the console line settings.
>>> However, if the tty is subsequently closed, the termios settings
>>> are lost. This results in a garbled console if the console is later
>>> suspended and resumed.
>>>
>>> Preserve the termios c_cflag for the serial console when the tty
>>> is shutdown; this reflects the most recent line settings.
>>>
>>> Fixes: Bugzilla #69751, 'serial console does not wake from S3'
>>> Reported-by: Valerio Vanni <[email protected]>
>>> Cc: Alan Cox <[email protected]>
>>> Signed-off-by: Peter Hurley <[email protected]>
>>> ---
>>> drivers/tty/serial/serial_core.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>
>> Same as before, regression, or just a normal "new feature"?
>
> This was reported as a regression since 2.6.24, but that was likely
> misreported, and more likely due to a userspace update which triggers
> the bug.
>
> I would be surprised if console resume _never_ worked if the serial tty
> was opened then closed, but it probably has worked for a long time.
^^^^^
hasn't
"Peter Hurley" <[email protected]> ha scritto nel messaggio
news:[email protected]
> On 06/04/2014 10:22 AM, Greg Kroah-Hartman wrote:
>> Same as before, regression, or just a normal "new feature"?
>
> This was reported as a regression since 2.6.24, but that was likely
> misreported, and more likely due to a userspace update which triggers
> the bug.
>
> I would be surprised if console resume _never_ worked if the serial
> tty was opened then closed, but it probably has worked for a long
> time.
I don't know what was my initial mistake, but I realized very soon it was
not a regression and took away the "regression" flag from bugzilla.
I tried many and many kernels and all were failing. I tried also to go
down from 2.6.24 version, but I had to stop rather early because at some
point the kernels were incompatible with distribution (I don't remember
the details since I did these tests some month ago, and later I tested
only newer kernel versions).
The patches you sent are for -next? for -rc? for stable?
Can I try them on 3.13.11? (at the moment I'm using this)
On Wed, 4 Jun 2014 10:16:10 -0400
Peter Hurley <[email protected]> wrote:
> If a tty is opened on a serial console, don't drop DTR on
> last tty close, on tty hangup, or when resetting port hardware
> via TIOCSSERIAL and TIOCSERCONFIG ioctls.
>
> Signed-off-by: Peter Hurley <[email protected]>
NAK
This introduces a security flaw.
If you have a system with a remote console you dial into then with this
patch applied a modem drop eg from a bad line will no longer drop any
live session and ensure a login is required as it was before.
That's a pretty bad regression case.
If you are running a serial console and want to leave DTR high either
wire the cable that way or don't set HUPCL in the first place. The
technology for fixing this problem already exists!
Alan
On 06/09/2014 09:08 AM, One Thousand Gnomes wrote:
> On Wed, 4 Jun 2014 10:16:10 -0400
> Peter Hurley <[email protected]> wrote:
>
>> If a tty is opened on a serial console, don't drop DTR on
>> last tty close, on tty hangup, or when resetting port hardware
>> via TIOCSSERIAL and TIOCSERCONFIG ioctls.
>>
>> Signed-off-by: Peter Hurley <[email protected]>
>
> NAK
>
> This introduces a security flaw.
>
> If you have a system with a remote console you dial into then with this
> patch applied a modem drop eg from a bad line will no longer drop any
> live session and ensure a login is required as it was before.
There's no security flaw here.
The situation you're referring to above is managed by the CLOCAL termios
setting (which by default does _not_ hangup the tty on carrier loss).
This patch only affects the line state if the last tty reference is closed
or the tty is hung up by software (like on controlling process exit).
In this case, any login session is already dying, and it would not be possible
to hijack a live session. A successful re-login is still required.
> That's a pretty bad regression case.
>
> If you are running a serial console and want to leave DTR high either
> wire the cable that way or don't set HUPCL in the first place. The
> technology for fixing this problem already exists!
Notwithstanding what I wrote above, this patch does change behavior
with remote consoles, which may be unacceptable.
For example, if the remote user logs out, the current behavior hangs up
the modem (if HUPCL), whereas the patch behavior just presents a new
login prompt.
So yeah, I agree; this patch should be dropped.
Regards,
Peter Hurley
> This patch only affects the line state if the last tty reference is closed
> or the tty is hung up by software (like on controlling process exit).
> In this case, any login session is already dying, and it would not be possible
> to hijack a live session. A successful re-login is still required.
It breaks the other direction - yes sorry.
> Notwithstanding what I wrote above, this patch does change behavior
> with remote consoles, which may be unacceptable.
> For example, if the remote user logs out, the current behavior hangs up
> the modem (if HUPCL), whereas the patch behavior just presents a new
> login prompt.
which means the modem will probably no longer answer calls.
> So yeah, I agree; this patch should be dropped.
Possibly we should also fix this the other way in tty_port_shutdown,
which does wrongly skip dtr/rts handling of consoles.
As far as I can see providing HUPCL is clear the desired console
behaviour for most situations with a remote console link is obtained.
Alan
On 06/10/2014 07:01 AM, One Thousand Gnomes wrote:
>> This patch only affects the line state if the last tty reference is closed
>> or the tty is hung up by software (like on controlling process exit).
>> In this case, any login session is already dying, and it would not be possible
>> to hijack a live session. A successful re-login is still required.
>
> It breaks the other direction - yes sorry.
>
>> Notwithstanding what I wrote above, this patch does change behavior
>> with remote consoles, which may be unacceptable.
>
>> For example, if the remote user logs out, the current behavior hangs up
>> the modem (if HUPCL), whereas the patch behavior just presents a new
>> login prompt.
>
> which means the modem will probably no longer answer calls.
I would expect the vast majority of modems to be configured to hang-up
after carrier loss (when the remote user hangs up).
I think the more significant impact is that killing the login process
(by the sysadmin) wouldn't disconnect a malicious user (but would still
force the re-login). I see this as more nuisance value than any
realistic DoS.
>> So yeah, I agree; this patch should be dropped.
>
> Possibly we should also fix this the other way in tty_port_shutdown,
> which does wrongly skip dtr/rts handling of consoles.
>
> As far as I can see providing HUPCL is clear the desired console
> behaviour for most situations with a remote console link is obtained.
I agree that serial core and tty port should handle the console
identically, so if the serial core defaults are the expected behavior,
then, yes, tty_port_shutdown() should not skip dropping dtr/rts.
However, a console on USB tty is much more likely to be local, rather
than remote, so changing dtr/rts handling could impact these setups.
Since the termios c_cflag is inherited from the console, what about adding
new flow control flag decodes to uart_set_options(); eg.,
'r' = CRTSCTS | HUPCL /* existing */
'R' = CRTSCTS | !HUPCL
default = !CRTSCTS | HUPCL
'd' = !CRTSCTS | !HUPCL
?
Regards,
Peter Hurley