linux-kernel - RE: [PATCH v7 3/9] software node: allow referencing firmware nodes

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: <030101dc73f1$46a62b40$d3f281c0$@trustnetic.com>
Date: Tue, 23 Dec 2025 17:48:22 +0800
From: Jiawen Wu <jiawenwu@...stnetic.com>
To: "'Bartosz Golaszewski'" <brgl@...nel.org>
Cc: "'Bartosz Golaszewski'" <bartosz.golaszewski@...aro.org>,
	<andriy.shevchenko@...ux.intel.com>,
	<andy@...nel.org>,
	<broonie@...nel.org>,
	<ckeepax@...nsource.cirrus.com>,
	<dakr@...nel.org>,
	<david.rhodes@...rus.com>,
	<djrscally@...il.com>,
	<gregkh@...uxfoundation.org>,
	<heikki.krogerus@...ux.intel.com>,
	<krzk@...nel.org>,
	<linus.walleij@...aro.org>,
	<linux-acpi@...r.kernel.org>,
	<linux-gpio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<linux-sound@...r.kernel.org>,
	<linux-spi@...r.kernel.org>,
	<mstrozek@...nsource.cirrus.com>,
	<p.zabel@...gutronix.de>,
	<patches@...nsource.cirrus.com>,
	<rafael@...nel.org>,
	<rf@...nsource.cirrus.com>,
	<sakari.ailus@...ux.intel.com>
Subject: RE: [PATCH v7 3/9] software node: allow referencing firmware nodes

On Tue, Dec 23, 2025 5:37 PM, Bartosz Golaszewski wrote:
> On Tue, Dec 23, 2025 at 9:44 AM Jiawen Wu <jiawenwu@...stnetic.com> wrote:
> >
> > On Tue, Dec 23, 2025 4:09 PM, Bartosz Golaszewski wrote:
> > > On Tue, Dec 23, 2025 at 8:39 AM Jiawen Wu <jiawenwu@...stnetic.com> wrote:
> > > >
> > > > >
> > > > >  #define SOFTWARE_NODE_REFERENCE(_ref_, ...)                  \
> > > > >  (const struct software_node_ref_args) {                              \
> > > > > -     .node = _ref_,                                          \
> > > > > +     .swnode = _Generic(_ref_,                               \
> > > > > +                        const struct software_node *: _ref_, \
> > > > > +                        default: NULL),                      \
> > > > > +     .fwnode = _Generic(_ref_,                               \
> > > > > +                        struct fwnode_handle *: _ref_,       \
> > > > > +                        default: NULL),                      \
> > > > >       .nargs = COUNT_ARGS(__VA_ARGS__),                       \
> > > > >       .args = { __VA_ARGS__ },                                \
> > > > >  }
> > > >
> > > > This change seems incompatible with my driver txgbe, since the software nodes
> > > > are registered in " struct software_node * " but not " const struct software_node * ".
> > > >
> > > > So when I pulled the net-next-6.19-rc1 that merged this patch, to probe my driver.
> > > > The error logs shows:
> > > >
> > > > [    5.243396] txgbe 0000:10:00.0 (unnamed net_device) (uninitialized): unable to attach SFP bus: -EINVAL
> > > > [    5.243399] txgbe 0000:10:00.0: failed to init phylink
> > > > [    5.576008] txgbe 0000:10:00.0: probe with driver txgbe failed with error -22
> > > > [    6.109548] txgbe 0000:10:00.1 (unnamed net_device) (uninitialized): unable to attach SFP bus: -EINVAL
> > > > [    6.109551] txgbe 0000:10:00.1: failed to init phylink
> > > > [    6.442044] txgbe 0000:10:00.1: probe with driver txgbe failed with error -22
> > > >
> > >
> > > This shouldn't have changed anything for existing software nodes - the
> > > pointer in struct software_node_ref_args has always been const. This
> > > would have failed at build-time if this was an issue. Have you
> > > bisected it to this very commit? What line is the -EINVAL assigned and
> > > for what reason?
> >
> > The -EINVAL is assigned software_node_get_reference_args():
> >
> >         if (ref->swnode)
> >                 refnode = software_node_fwnode(ref->swnode);
> >         else if (ref->fwnode)
> >                 refnode = ref->fwnode;
> >         else
> >                 return -EINVAL;
> >
> > I think the reason is that _Generic selection is restrictive, it only accept
> > exactly const struct software_node for software node references. So the
> > macro SOFTWARE_NODE_REFERENCE fall back to the default to set .swnode =  NULL.
> >
> > And I temporarily added this line to fix it:
> >
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index 272bfbdea7bf..e30ef23a9af3 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -371,6 +371,7 @@ struct software_node_ref_args {
> >  (const struct software_node_ref_args) {                                \
> >         .swnode = _Generic(_ref_,                               \
> >                            const struct software_node *: _ref_, \
> > +                          struct software_node *: _ref_,       \
> >                            default: NULL),                      \
> >         .fwnode = _Generic(_ref_,                               \
> >                            struct fwnode_handle *: _ref_,       \
> >
> 
> Ah I see, we'd assign struct software_node * to const struct
> software_node * and it used to work but with _Generic() we need the
> exact type. I agree with this approach, do you want to send a proper
> patch?

It might be more appropriate for you to send the patch, and could also
check if there are any other missed details, like for fwnode...
I'm not very proficient in this field. :)