[PATCH v4 0/2] of: property: Fix potential firmware node reference's argument count got out of range

[PATCH v4 0/2] of: property: Fix potential firmware node reference's argument count got out of range

[Zijun Hu]

This patch series is to fix an OF issue below:

The firmware node reference argument count @args->nargs got by
of_fwnode_get_reference_args() may be out of range.

---
Changes in v4:
- Correct commit message for patch 1/2
- Add one more patch 2/2 to align both macros
- Link to v3: https://lore.kernel.org/r/20250210-fix_arg_count-v3-1-a084a5013008@quicinc.com

Changes in v3:
- Remove RFC, correct title and commit message.
- Link to v2: https://lore.kernel.org/r/20250114-fix_arg_count-v2-1-efa35ee6572b@quicinc.com

Changes in v2:
- Increase macro @NR_FWNODE_REFERENCE_ARGS to align with @MAX_PHANDLE_ARGS.
- Correct fix tag and send as RFC patch.
- Link to v1: https://lore.kernel.org/r/20250109-of_core_fix-v4-7-db8a72415b8c@quicinc.com

---
Zijun Hu (2):
      of: property: Increase NR_FWNODE_REFERENCE_ARGS
      of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

 include/linux/fwnode.h | 2 +-
 include/linux/of.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
---
base-commit: cd726b80a9b2e35397f3f4ecd845a8fd247ee79e
change-id: 20250114-fix_arg_count-73feae90fccb

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

[PATCH v4 1/2] of: property: Increase NR_FWNODE_REFERENCE_ARGS

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

Currently, the following two macros have different values:

// The maximal argument count for firmware node reference
 #define NR_FWNODE_REFERENCE_ARGS	8
// The maximal argument count for DT node reference
 #define MAX_PHANDLE_ARGS 16

It may cause firmware node reference's argument count out of range if
directly assign DT node reference's argument count to firmware's.

drivers/of/property.c:of_fwnode_get_reference_args() is doing the direct
assignment, so may cause firmware's argument count @args->nargs got out
of range, namely, in [9, 16].

Fix by increasing NR_FWNODE_REFERENCE_ARGS to 16 to meet DT requirement.
Will align both macros later to avoid such inconsistency.

Fixes: 3e3119d3088f ("device property: Introduce fwnode_property_get_reference_args")
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 include/linux/fwnode.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 0731994b9d7c832cae8a30063f3a64194e4f19aa..6fa0a268d53827a376d7f258c6194a2a088e4325 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -91,7 +91,7 @@ struct fwnode_endpoint {
 #define SWNODE_GRAPH_PORT_NAME_FMT		"port@%u"
 #define SWNODE_GRAPH_ENDPOINT_NAME_FMT		"endpoint@%u"
 
-#define NR_FWNODE_REFERENCE_ARGS	8
+#define NR_FWNODE_REFERENCE_ARGS	16
 
 /**
  * struct fwnode_reference_args - Fwnode reference with additional arguments

-- 
2.34.1

[PATCH v4 2/2] of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

Macro NR_FWNODE_REFERENCE_ARGS defines the maximal argument count
for firmware node reference, and MAX_PHANDLE_ARGS defines the maximal
argument count for DT node reference, both have the same value now.

To void argument count inconsistency between firmware and DT, simply
align both macros by '#define MAX_PHANDLE_ARGS NR_FWNODE_REFERENCE_ARGS'.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 include/linux/of.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index eaf0e2a2b75cbe4769a77f68965298c84d57e62c..86bf8f0731112456b3efc2e5ee00e73000c6af56 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -67,7 +67,7 @@ struct device_node {
 #endif
 };
 
-#define MAX_PHANDLE_ARGS 16
+#define MAX_PHANDLE_ARGS NR_FWNODE_REFERENCE_ARGS
 struct of_phandle_args {
 	struct device_node *np;
 	int args_count;

-- 
2.34.1

Re: [PATCH v4 2/2] of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

[Andy Shevchenko]

On Tue, Feb 25, 2025 at 09:58:07PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Macro NR_FWNODE_REFERENCE_ARGS defines the maximal argument count
> for firmware node reference, and MAX_PHANDLE_ARGS defines the maximal
> argument count for DT node reference, both have the same value now.
> 
> To void argument count inconsistency between firmware and DT, simply
> align both macros by '#define MAX_PHANDLE_ARGS NR_FWNODE_REFERENCE_ARGS'.

I would add here that the of.h includes fwnode.h already, so it doesn't
add any new compile time dependency.

Both patches LGTM,
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/2] of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

[Sakari Ailus]

Hi Andy, Zijun,

On Tue, Feb 25, 2025 at 04:20:34PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 25, 2025 at 09:58:07PM +0800, Zijun Hu wrote:
> > From: Zijun Hu <quic_zijuhu@quicinc.com>
> > 
> > Macro NR_FWNODE_REFERENCE_ARGS defines the maximal argument count
> > for firmware node reference, and MAX_PHANDLE_ARGS defines the maximal
> > argument count for DT node reference, both have the same value now.
> > 
> > To void argument count inconsistency between firmware and DT, simply
> > align both macros by '#define MAX_PHANDLE_ARGS NR_FWNODE_REFERENCE_ARGS'.
> 
> I would add here that the of.h includes fwnode.h already, so it doesn't
> add any new compile time dependency.

It's included via property.h but I think that's fine.

> 
> Both patches LGTM,
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
Sakari Ailus

Re: [PATCH v4 1/2] of: property: Increase NR_FWNODE_REFERENCE_ARGS

[Sakari Ailus]

On Tue, Feb 25, 2025 at 09:58:06PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Currently, the following two macros have different values:
> 
> // The maximal argument count for firmware node reference
>  #define NR_FWNODE_REFERENCE_ARGS	8
> // The maximal argument count for DT node reference
>  #define MAX_PHANDLE_ARGS 16
> 
> It may cause firmware node reference's argument count out of range if
> directly assign DT node reference's argument count to firmware's.
> 
> drivers/of/property.c:of_fwnode_get_reference_args() is doing the direct
> assignment, so may cause firmware's argument count @args->nargs got out
> of range, namely, in [9, 16].
> 
> Fix by increasing NR_FWNODE_REFERENCE_ARGS to 16 to meet DT requirement.
> Will align both macros later to avoid such inconsistency.
> 
> Fixes: 3e3119d3088f ("device property: Introduce fwnode_property_get_reference_args")
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

Re: [PATCH v4 2/2] of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

[Sakari Ailus]

On Tue, Feb 25, 2025 at 09:58:07PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Macro NR_FWNODE_REFERENCE_ARGS defines the maximal argument count
> for firmware node reference, and MAX_PHANDLE_ARGS defines the maximal
> argument count for DT node reference, both have the same value now.
> 
> To void argument count inconsistency between firmware and DT, simply
> align both macros by '#define MAX_PHANDLE_ARGS NR_FWNODE_REFERENCE_ARGS'.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

Re: [PATCH v4 2/2] of: Align macro MAX_PHANDLE_ARGS with NR_FWNODE_REFERENCE_ARGS

[Zijun Hu]

On 2025/2/25 22:20, Andy Shevchenko wrote:
> I would add here that the of.h includes fwnode.h already, so it doesn't
> add any new compile time dependency.
> 

thank you Andy for comments.

include/linux/of.h:
struct device_node {
...
	struct fwnode_handle fwnode; // this type is defined by fwnode.h
...
}

include/linux/fwnode.h:
struct fwnode_handle {...}

So of.h must include fwnode.h


Perhaps, of.h needs to include fwnode.h *explicitly* even if this is
another topic.

> Both patches LGTM,
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>