linux-kernel - Re: [RFC PATCH v1 12/12] usb: typec: qcom: define the bridge's path

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]

Date:   Mon, 30 Oct 2023 10:19:08 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Janne Grunau <j@...nau.net>, Simon Ser <contact@...rsion.fr>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        freedreno@...ts.freedesktop.org
Subject: Re: [RFC PATCH v1 12/12] usb: typec: qcom: define the bridge's path

On Mon, Oct 23, 2023 at 09:24:33PM +0300, Dmitry Baryshkov wrote:
> On 15 September 2023 15:14:35 EEST, Heikki Krogerus <heikki.krogerus@...ux.intel.com> wrote:
> >Hi Dmitry,
> >
> >On Mon, Sep 04, 2023 at 12:41:50AM +0300, Dmitry Baryshkov wrote:
> >> In order to notify the userspace about the DRM connector's USB-C port,
> >> export the corresponding port's name as the bridge's path field.
> >> 
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> >> ---
> >>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c     | 11 +++++++----
> >>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c |  4 +++-
> >>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h |  6 ++++--
> >>  3 files changed, 14 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> >> index b9d4856101c7..452dc6437861 100644
> >> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> >> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> >> @@ -156,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
> >>  	struct device_node *np = dev->of_node;
> >>  	const struct pmic_typec_resources *res;
> >>  	struct regmap *regmap;
> >> +	char *tcpm_name;
> >>  	u32 base[2];
> >>  	int ret;
> >>  
> >> @@ -211,10 +212,6 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
> >>  	mutex_init(&tcpm->lock);
> >>  	platform_set_drvdata(pdev, tcpm);
> >>  
> >> -	tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev);
> >> -	if (IS_ERR(tcpm->pmic_typec_drm))
> >> -		return PTR_ERR(tcpm->pmic_typec_drm);
> >> -
> >>  	tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
> >>  	if (!tcpm->tcpc.fwnode)
> >>  		return -EINVAL;
> >> @@ -225,6 +222,12 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
> >>  		goto fwnode_remove;
> >>  	}
> >>  
> >> +	tcpm_name = tcpm_port_get_name(tcpm->tcpm_port);
> >> +	tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev, tcpm_name);
> >
> >So I got some questions and concerns off-list. This was one of the
> >concerns. That tcpm_name is now the actual port device name, so I'm
> >afraid this is not acceptable.
> >
> >You can't use device name as a reference, ever. There is no way to
> >guarantee that a device with a specific name is what you meant it to
> >be by the time it's accessed.
> 
> Hmm, could you please be more specific, why? I mean, class devices are not
> that easy to be renamed in sysfs, are they? Or are you concerned about the
> device being destroyed behind userspace's back? At least for MSM this will be
> a huge problem already, with the bridge driver suddenly being removed.

The race exists even in your case, but please do not look at this as a
solution for only your platform.

This is about showing the user space a link between two device
instances (struct device), and the way you do that is by creating a
symlink. That way the kernel can take care of reference counting and
guarantee that the link always points to the correct device. That way
the link will also be always visible in user space without requirement
for any specific ABI like it should.

thanks,

-- 
heikki