linux-kernel - Re: [PATCH v2 1/7] serial: qcom-geni: fix premature receiver enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ocfbiqmspqlulnxjs7lmmdyq65f2u5dogksqqkmhdq55m3gqyj@7ryn4vrjzemc>
Date: Tue, 1 Oct 2024 21:07:00 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
Cc: Johan Hovold <johan+linaro@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, Douglas Anderson <dianders@...omium.org>, 
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org, 
	stable@...r.kernel.org, Aniket Randive <quic_arandive@...cinc.com>
Subject: Re: [PATCH v2 1/7] serial: qcom-geni: fix premature receiver enable

On Tue, Oct 01, 2024 at 07:20:36PM GMT, Mukesh Kumar Savaliya wrote:
> Thanks Johan for the fixes.
> 
> On 10/1/2024 6:20 PM, Johan Hovold wrote:
> > The receiver should not be enabled until the port is opened so drop the
> > bogus call to start rx from the setup code which is shared with the
> > console implementation.
> > 
> > This was added for some confused implementation of hibernation support,
> > but the receiver must not be started unconditionally as the port may not
> > have been open when hibernating the system.
> > 
> > Fixes: 35781d8356a2 ("tty: serial: qcom-geni-serial: Add support for Hibernation feature")
> > Cc:stable@...r.kernel.org	# 6.2
> > Cc: Aniket Randive<quic_arandive@...cinc.com>
> > Signed-off-by: Johan Hovold<johan+linaro@...nel.org>
> > ---
> >   drivers/tty/serial/qcom_geni_serial.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 6f0db310cf69..9ea6bd09e665 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1152,7 +1152,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
> >   			       false, true, true);
> >   	geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
> >   	geni_se_select_mode(&port->se, port->dev_data->mode);
> > -	qcom_geni_serial_start_rx(uport);
> Does it mean hibernation will break now ? Not sure if its tested with
> hibernation. I can see this call was added to port_setup specifically for
> hibernation but now after removing it, where is it getting fixed ?

Can you explain how you're testing hibernation and on which platform
this is done? I'd like to add this to my set of tests, but last time I
tested I couldn't find a platform where we survived the restore
processes (it's been a while though).

> I think RX will not be initialized after hibernation.

qcom_geni_serial_port_setup() is invoked in multiple places, how come
we don't perform this hibernation-specific operation in
qcom_geni_serial_sys_hib_resume()? (And why is it called hib_resume when
the kernel nomenclature for what it does is "restore"?)

Regards,
Bjorn

> >   	port->setup = true;
> >   	return 0;
> > -- 2.45.2