Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Doug Berger <opendmb@gmail.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        Jason Wessel <jason.wessel@windriver.com>,
        Florian Fainelli <florian.fainelli@broadcom.com>,
        Doug Berger <opendmb@gmail.com>
Subject: [RFC PATCH] tty: keep last tty attached to console ports
Date:   Fri, 21 Jul 2023 14:17:23 -0700
Message-Id: <20230721211723.4082643-1-opendmb@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

I have observed an issue when resuming a UART device that
supports a console where the serial_core attempts to set the
baud rate to 0.

The scenario is as follows:
The systemd software suite intentionally opens and closes the
/dev/console for each status line. When the console is on
/dev/ttyS0 this has the effect that any initial console
configuration is migrated to the first tty_struct associated
with the port and the console configuration is zeroed out in
uart_port_startup(). When the UART is closed tty_port_close()
is invoked and if no other device beside the console is open on
the port tty_port_shutdown() is invoked. However, there is
explicit logic in tty_port_shutdown() that avoids invoking the
shutdown functions of the port when it is a console. The final
step of tty_port_close() is to disconnect the tty_struct from
the port which causes the console configuration information to
be lost. If the device is suspended and resumed after this point
uart_resume_port() will attempt to recover the configuration
data from the console structure that was previously zeroed out
and since the tty_struct was disconnected the set_termios()
operation of the driver will be invoked with the zeroed termios.
In many cases this will lead to setting the baud rate to a
default 9600bps which is often undesired. In other cases it may
lead to division by 0 or something else depending on the
underlying driver.

It would seem more appropriate to me for tty_port_shutdown() to
invoke the shutdown functions of the underlying port so that the
serial core layer would have the opportunity to preserve the
current termios settings back into the console structure. There
is logic in uart_shutdown() to do this, but in this case it
would be more appropriate for that code to be moved to
uart_port_shutdown() instead.

However, the logic of tty_port_shutdown() appears to have been
explicitly changed to have the current behavior of not calling
shutdown operations when the tty port is a console to prevent
another issue (I believe in ttyUSB) so I don't want to propose
this solution without a better understanding of why that change
was made.

As an alternative, the tty_port_close() function could leave the
last tty_struct connected to console ports as suggested here.
This would allow uart_resume_port() to recover the termios
settings from there.

Please help me find the best way to address this issue.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/tty/tty_port.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index a788a6bf487d..18c96b153815 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -721,7 +721,8 @@ void tty_port_close(struct tty_port *port, struct tty_struct *tty,
 	if (!port->console)
 		set_bit(TTY_IO_ERROR, &tty->flags);
 	tty_port_close_end(port, tty);
-	tty_port_tty_set(port, NULL);
+	if (!port->console)
+		tty_port_tty_set(port, NULL);
 }
 EXPORT_SYMBOL(tty_port_close);
 
-- 
2.34.1