linux-kernel - Re: [PATCH v2] usb: typec: qcom-pmic-typec: split HPD bridge alloc and registration

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: <ZhPYjXjX3LcCMhyh@hovoldconsulting.com>
Date: Mon, 8 Apr 2024 13:44:13 +0200
From: Johan Hovold <johan@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Caleb Connolly <caleb.connolly@...aro.org>
Subject: Re: [PATCH v2] usb: typec: qcom-pmic-typec: split HPD bridge alloc
 and registration

On Mon, Apr 08, 2024 at 01:49:48PM +0300, Dmitry Baryshkov wrote:
> On Mon, Apr 08, 2024 at 09:11:32AM +0200, Johan Hovold wrote:
> > On Mon, Apr 08, 2024 at 04:06:40AM +0300, Dmitry Baryshkov wrote:
> > > If a probe function returns -EPROBE_DEFER after creating another device
> > > there is a change of ending up in a probe deferral loop, (see commit
> > > fbc35b45f9f6 ("Add documentation on meaning of -EPROBE_DEFER").
> > > 
> > > In order to prevent such probe-defer loops caused by qcom-pmic-typec
> > > driver, use the API added by Johan Hovold and move HPD bridge
> > > registration to the end of the probe function.
> > 
> > You should be more specific here: which function called after
> > qcom_pmic_typec_probe() can trigger a probe deferral?
> > 
> > I doubt that applies to tcpm->port_start() and tcpm->pdphy_start() in
> > which case the bridge should be added before those calls unless there
> > are other reasons for not doing so, which then also should be mentioned.
> > 
> > I suspect the trouble is with tcpm_register_port(), but please spell
> > that out and mention in which scenarios that function may return
> > -EPROBE_DEFER.
> 
> The probe loop comes from from tcpm_register_port(), you are right.
> However then putting bridge registration before the _start() functions
> is also incorrect as this will be prone to use-after-free errors that
> you have fixed in pmic-glink.

You obviously have to mention that in the commit message as that is a
separate change and also one that looks broken as you're now registering
resources after the device has gone "live".

So you also need to explain why you think that is safe, if it should be
done at all. You're essentially just papering over a DRM bug in the
unlikely event that probe fails.

Johan