linux-kernel - Re: [PATCH] platform/x86: intel: chtwc_int33fe: don't dereference swnode args

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: <c116bb9d-1c56-abf3-a9b7-c87dbcc63523@linux.intel.com>
Date: Fri, 21 Nov 2025 16:18:11 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Hans de Goede <hansg@...nel.org>
cc: Bartosz Golaszewski <brgl@...ev.pl>, 
    Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
    Sakari Ailus <sakari.ailus@...ux.intel.com>, 
    Philipp Zabel <p.zabel@...gutronix.de>, 
    Linus Walleij <linus.walleij@...aro.org>, 
    Dmitry Torokhov <dmitry.torokhov@...il.com>, 
    "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, 
    Charles Keepax <ckeepax@...nsource.cirrus.com>, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, 
    Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, 
    stable@...r.kernel.org, Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH] platform/x86: intel: chtwc_int33fe: don't dereference
 swnode args

On Fri, 21 Nov 2025, Hans de Goede wrote:

> Hi,
> 
> On 21-Nov-25 11:04 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > 
> > Members of struct software_node_ref_args should not be dereferenced
> > directly but set using the provided macros. Commit d7cdbbc93c56
> > ("software node: allow referencing firmware nodes") changed the name of
> > the software node member and caused a build failure. Remove all direct
> > dereferences of the ref struct as a fix.
> > 
> > However, this driver also seems to abuse the software node interface by
> > waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear
> > in the system before setting up the reference for the I2C device, while
> > the actual software node already exists in the intel-xhci-usb-role-switch
> > module and should be used to set up a static reference. Add a FIXME for
> > a future improvement.
> > 
> > Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes")
> > Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties")
> > Cc: stable@...r.kernel.org
> > Reported-by: Stephen Rothwell <sfr@...b.auug.org.au>
> > Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > ---
> > This should go into the reset tree as a fix to the regression introduced
> > by the reset-gpio driver rework.
> 
> Thanks, patch looks good to me:
> 
> Reviewed-by: Hans de Goede <johannes.goede@....qualcomm.com>
> 
> Also ack for merging this through the reset tree.
> 
> Ilpo please do *not* pick this one up as it will be merged
> through the reset tree.

Fine,

Acked-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

-- 
 i.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> > ---
> >  drivers/platform/x86/intel/chtwc_int33fe.c | 29 ++++++++++++++++++++---------
> >  1 file changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/chtwc_int33fe.c b/drivers/platform/x86/intel/chtwc_int33fe.c
> > index 29e8b5432f4c9eea7dc45b83d94c0e00373f901b..d183aa53c318ba8d57c7124c38506e6956b3ee36 100644
> > --- a/drivers/platform/x86/intel/chtwc_int33fe.c
> > +++ b/drivers/platform/x86/intel/chtwc_int33fe.c
> > @@ -77,7 +77,7 @@ static const struct software_node max17047_node = {
> >   * software node.
> >   */
> >  static struct software_node_ref_args fusb302_mux_refs[] = {
> > -	{ .node = NULL },
> > +	SOFTWARE_NODE_REFERENCE(NULL),
> >  };
> >  
> >  static const struct property_entry fusb302_properties[] = {
> > @@ -190,11 +190,6 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
> >  {
> >  	software_node_unregister_node_group(node_group);
> >  
> > -	if (fusb302_mux_refs[0].node) {
> > -		fwnode_handle_put(software_node_fwnode(fusb302_mux_refs[0].node));
> > -		fusb302_mux_refs[0].node = NULL;
> > -	}
> > -
> >  	if (data->dp) {
> >  		data->dp->secondary = NULL;
> >  		fwnode_handle_put(data->dp);
> > @@ -202,7 +197,15 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
> >  	}
> >  }
> >  
> > -static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
> > +static void cht_int33fe_put_swnode(void *data)
> > +{
> > +	struct fwnode_handle *fwnode = data;
> > +
> > +	fwnode_handle_put(fwnode);
> > +	fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(NULL);
> > +}
> > +
> > +static int cht_int33fe_add_nodes(struct device *dev, struct cht_int33fe_data *data)
> >  {
> >  	const struct software_node *mux_ref_node;
> >  	int ret;
> > @@ -212,17 +215,25 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
> >  	 * until the mux driver has created software node for the mux device.
> >  	 * It means we depend on the mux driver. This function will return
> >  	 * -EPROBE_DEFER until the mux device is registered.
> > +	 *
> > +	 * FIXME: the relevant software node exists in intel-xhci-usb-role-switch
> > +	 * and - if exported - could be used to set up a static reference.
> >  	 */
> >  	mux_ref_node = software_node_find_by_name(NULL, "intel-xhci-usb-sw");
> >  	if (!mux_ref_node)
> >  		return -EPROBE_DEFER;
> >  
> > +	ret = devm_add_action_or_reset(dev, cht_int33fe_put_swnode,
> > +				       software_node_fwnode(mux_ref_node));
> > +	if (ret)
> > +		return ret;
> > +
> >  	/*
> >  	 * Update node used in "usb-role-switch" property. Note that we
> >  	 * rely on software_node_register_node_group() to use the original
> >  	 * instance of properties instead of copying them.
> >  	 */
> > -	fusb302_mux_refs[0].node = mux_ref_node;
> > +	fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(mux_ref_node);
> >  
> >  	ret = software_node_register_node_group(node_group);
> >  	if (ret)
> > @@ -345,7 +356,7 @@ static int cht_int33fe_typec_probe(struct platform_device *pdev)
> >  		return fusb302_irq;
> >  	}
> >  
> > -	ret = cht_int33fe_add_nodes(data);
> > +	ret = cht_int33fe_add_nodes(dev, data);
> >  	if (ret)
> >  		return ret;
> >  
> > 
> > ---
> > base-commit: cba510406ba76569782ead6007a0e4eb5d34a7ab
> > change-id: 20251121-int33fe-swnode-fix-e896da458560
> > 
> > Best regards,
>