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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZwaIk5MlqL3AL_qQ@hovoldconsulting.com>
Date: Wed, 9 Oct 2024 15:43:47 +0200
From: Johan Hovold <johan@...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>,
	Bjorn Andersson <andersson@...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 +0530, Mukesh Kumar Savaliya wrote:
> Thanks Johan for the fixes.

Thanks for taking a look.

> 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>

> > @@ -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 ?
> I think RX will not be initialized after hibernation.

Correct. As I alluded to in the commit message this "hibernation
support" is quite broken already, but I was trying to avoid spending
more time on this driver than I already have and just look the other way
for the time being.

Note that rx is enabled by the serial core resume code, but then this
hibernation hack added a call to the setup the port after resuming it,
which would disable rx again were it not for this random call to
start rx, which should never have been added here in the first place.

But as these platforms do not support hibernation in mainline, and the
code broken anyway, I'll just rip it all out for v3.

Johan