[PATCH 0/2] software node: bug fixes

[PATCH 0/2] software node: bug fixes

[Zijun Hu]

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Zijun Hu (2):
      software node: Add comment for the first ERR_CAST() in fwnode_create_software_node()
      software node: Correct a OOB check in software_node_get_reference_args()

 drivers/base/swnode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250410-fix_swnode-4986ff1b3534

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

[PATCH 1/2] software node: Add comment for the first ERR_CAST() in fwnode_create_software_node()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

ERR_CAST() is normally used to cast an error-valued pointer type to
another different type, But the first ERR_CAST() is to cast away the
const in fwnode_create_software_node().

Add comment for this unusual ERR_CAST() usage.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/swnode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index b1726a3515f6fbe13c2186af1f74479263798e42..67040fff99b02c43999b175c2ba7e6d04322a446 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -933,6 +933,7 @@ fwnode_create_software_node(const struct property_entry *properties,
 	struct software_node *node;
 	struct swnode *p;
 
+	/* Only cast away the const by ERR_CAST() */
 	if (IS_ERR(parent))
 		return ERR_CAST(parent);
 

-- 
2.34.1

Re: [PATCH 1/2] software node: Add comment for the first ERR_CAST() in fwnode_create_software_node()

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 09:12:11PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> ERR_CAST() is normally used to cast an error-valued pointer type to
> another different type, But the first ERR_CAST() is to cast away the
> const in fwnode_create_software_node().
> 
> Add comment for this unusual ERR_CAST() usage.

...

> +	/* Only cast away the const by ERR_CAST() */
>  	if (IS_ERR(parent))
>  		return ERR_CAST(parent);

TBH, I don't see the value of this comment. And looking into the code, I would
rather drop this part. The current users do not rely on the any specific code
to be returned and also they check parent to be valid beforehand.

But let's hear others first.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] software node: Add comment for the first ERR_CAST() in fwnode_create_software_node()

[Zijun Hu]

On 2025/4/14 16:00, Andy Shevchenko wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> ERR_CAST() is normally used to cast an error-valued pointer type to
>> another different type, But the first ERR_CAST() is to cast away the
>> const in fwnode_create_software_node().
>>
>> Add comment for this unusual ERR_CAST() usage.
> ...
> 
>> +	/* Only cast away the const by ERR_CAST() */
>>  	if (IS_ERR(parent))
>>  		return ERR_CAST(parent);
> TBH, I don't see the value of this comment. And looking into the code, I would

thank you Andy for comments.
will drop this patch in v2.

> rather drop this part. The current users do not rely on the any specific code
> to be returned and also they check parent to be valid beforehand.
> 
> But let's hear others first.

[PATCH 2/2] software node: Correct a OOB check in software_node_get_reference_args()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

software_node_get_reference_args() wants to get @index-th element, so
the property value requires at least '(index + 1) * sizeof(*ref)' bytes.

Correct the check to avoid OOB access.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/swnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 67040fff99b02c43999b175c2ba7e6d04322a446..efaac07f8ba38fae55214b71c2ecee15b5a711b1 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -529,7 +529,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
 	if (prop->is_inline)
 		return -EINVAL;
 
-	if (index * sizeof(*ref) >= prop->length)
+	if ((index + 1) * sizeof(*ref) > prop->length)
 		return -ENOENT;
 
 	ref_array = prop->pointer;

-- 
2.34.1

Re: [PATCH 2/2] software node: Correct a OOB check in software_node_get_reference_args()

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 09:12:12PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> software_node_get_reference_args() wants to get @index-th element, so
> the property value requires at least '(index + 1) * sizeof(*ref)' bytes.
> 
> Correct the check to avoid OOB access.

Any real traceback?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] software node: Correct a OOB check in software_node_get_reference_args()

[Zijun Hu]

On 2025/4/14 16:08, Andy Shevchenko wrote:
> On Thu, Apr 10, 2025 at 09:12:12PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> software_node_get_reference_args() wants to get @index-th element, so
>> the property value requires at least '(index + 1) * sizeof(*ref)' bytes.
>>
>> Correct the check to avoid OOB access.
> Any real traceback?

no, find this issue during reading code.

Re: [PATCH 2/2] software node: Correct a OOB check in software_node_get_reference_args()

[Andy Shevchenko]

On Mon, Apr 14, 2025 at 07:12:27PM +0800, Zijun Hu wrote:
> On 2025/4/14 16:08, Andy Shevchenko wrote:
> > On Thu, Apr 10, 2025 at 09:12:12PM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <quic_zijuhu@quicinc.com>
> >>
> >> software_node_get_reference_args() wants to get @index-th element, so
> >> the property value requires at least '(index + 1) * sizeof(*ref)' bytes.
> >>
> >> Correct the check to avoid OOB access.
> > Any real traceback?
> 
> no, find this issue during reading code.

Please, mention this in the commit message.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] software node: Correct a OOB check in software_node_get_reference_args()

[Sakari Ailus]

On Thu, Apr 10, 2025 at 09:12:12PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> software_node_get_reference_args() wants to get @index-th element, so
> the property value requires at least '(index + 1) * sizeof(*ref)' bytes.
> 
> Correct the check to avoid OOB access.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

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

> ---
>  drivers/base/swnode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 67040fff99b02c43999b175c2ba7e6d04322a446..efaac07f8ba38fae55214b71c2ecee15b5a711b1 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -529,7 +529,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
>  	if (prop->is_inline)
>  		return -EINVAL;
>  
> -	if (index * sizeof(*ref) >= prop->length)
> +	if ((index + 1) * sizeof(*ref) > prop->length)
>  		return -ENOENT;
>  
>  	ref_array = prop->pointer;
> 
> -- 
> 2.34.1
> 

-- 
Sakari Ailus

Re: [PATCH 0/2] software node: bug fixes

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 09:12:10PM +0800, Zijun Hu wrote:

If it's a series, cover letter is on purpose. Please, explain what's going on here.

> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

-- 
With Best Regards,
Andy Shevchenko