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: <CAMRc=Meugd9tEDefPnYHidDMTdCP+8fptVXNvqjSi1tjXPuVRA@mail.gmail.com>
Date: Tue, 23 Dec 2025 10:36:49 +0100
From: Bartosz Golaszewski <brgl@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
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 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?

Bart