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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 20 Feb 2024 18:40:40 -0800
From: Saravana Kannan <saravanak@...gle.com>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Rob Herring <robh+dt@...nel.org>, Frank Rowand <frowand.list@...il.com>, 
	Shawn Guo <shawnguo@...nel.org>, Wolfram Sang <wsa@...nel.org>, Mark Brown <broonie@...nel.org>, 
	Geert Uytterhoeven <geert+renesas@...der.be>, Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, stable@...r.kernel.org, 
	Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH 2/2] of: property: fw_devlink: Fix links to supplier when
 created from phandles

On Tue, Feb 20, 2024 at 3:10 AM Herve Codina <herve.codina@...tlin.com> wrote:
>
> Since commit 1a50d9403fb9 ("treewide: Fix probing of devices in DT
> overlays"), when using device-tree overlays, the FWNODE_FLAG_NOT_DEVICE
> is set on each overlay nodes. This flag is cleared when a struct device
> is actually created for the DT node.
> Also, when a device is created, the device DT node is parsed for known
> phandle and devlinks consumer/supplier links are created between the
> device (consumer) and the devices referenced by phandles (suppliers).
> As these supplier device can have a struct device not already created,
> the FWNODE_FLAG_NOT_DEVICE can be set for suppliers and leads the
> devlink supplier point to the device's parent instead of the device
> itself.
>
> Avoid this situation clearing the supplier FWNODE_FLAG_NOT_DEVICE just
> before the devlink creation if a device is supposed to be created and
> handled later in the process.
>
> Fixes: 1a50d9403fb9 ("treewide: Fix probing of devices in DT overlays")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Herve Codina <herve.codina@...tlin.com>
> ---
>  drivers/of/property.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 641a40cf5cf3..ff5cac477dbe 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1097,6 +1097,7 @@ static void of_link_to_phandle(struct device_node *con_np,
>                               struct device_node *sup_np)
>  {
>         struct device_node *tmp_np = of_node_get(sup_np);
> +       struct fwnode_handle *sup_fwnode;
>
>         /* Check that sup_np and its ancestors are available. */
>         while (tmp_np) {
> @@ -1113,7 +1114,20 @@ static void of_link_to_phandle(struct device_node *con_np,
>                 tmp_np = of_get_next_parent(tmp_np);
>         }
>
> -       fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np));
> +       /*
> +        * In case of overlays, the fwnode are added with FWNODE_FLAG_NOT_DEVICE
> +        * flag set. A node can have a phandle that references an other node
> +        * added by the overlay.
> +        * Clear the supplier's FWNODE_FLAG_NOT_DEVICE so that fw_devlink links
> +        * to this supplier instead of linking to its parent.
> +        */
> +       sup_fwnode = of_fwnode_handle(sup_np);
> +       if (sup_fwnode->flags & FWNODE_FLAG_NOT_DEVICE) {
> +               if (of_property_present(sup_np, "compatible") &&
> +                   of_device_is_available(sup_np))
> +                       sup_fwnode->flags &= ~FWNODE_FLAG_NOT_DEVICE;
> +       }
> +       fwnode_link_add(of_fwnode_handle(con_np), sup_fwnode);

Nack.

of_link_to_phandle() doesn't care about any of the fwnode flags. It
just creates links between the consumer and supplier nodes. Don't add
more intelligence into it please. Also, "compatible" doesn't really
guarantee device creation and you can have devices created out of
nodes with no compatible property. I finally managed to get away from
looking for the "compatible" property. So, let's not add back a
dependency on that property please.

Can you please give a real example where you are hitting this? I have
some thoughts on solutions, but I want to understand the issue fully
before I make suggestions.

Thanks,
Saravana

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ