[PATCH v2 0/2] HID: sensor: custom: Fix fields lifetime issues

[PATCH v2 0/2] HID: sensor: custom: Fix fields lifetime issues

[Haoxiang Li]

Hi,

This series fixes lifetime issues around sensor_inst->fields and the
sysfs attributes that can access it.

The first patch fixes the remove path by removing enable_sensor before
freeing the field attributes. enable_sensor_store() can dereference
power_state and report_state, which point into sensor_inst->fields.

The second patch fixes the original field sysfs group leak on probe
failure. It creates the field attributes before exposing enable_sensor,
then unwinds any field groups that were created before a later
sysfs_create_group() failure.

Thanks, Jiri, for the review and for pointing out the UAF concern.

Changes in v2:
- Split the fix into two patches.
- Fix the pre-existing remove path ordering issue.
- Create field attributes before exposing enable_sensor.
- Unwind already-created field sysfs groups on failure.

Haoxiang Li (2):
  HID: sensor: custom: Remove enable_sensor before freeing fields
  HID: sensor: custom: Fix field sysfs group cleanup on failure

 drivers/hid/hid-sensor-custom.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)


base-commit: ef0c9f75a19532d7675384708fc8621e10850104
-- 
2.25.1

[PATCH v2 1/2] HID: sensor: custom: Remove enable_sensor before freeing fields

[Haoxiang Li]

enable_sensor_store() can call set_power_report_state(), which
dereferences sensor_inst->power_state and sensor_inst->report_state.
These pointers refer to entries in sensor_inst->fields.

hid_sensor_custom_remove() currently frees the field attributes before
removing the enable_sensor sysfs attribute, leaving a window where a
concurrent sysfs write can dereference freed memory.

Remove enable_sensor before freeing the field attributes.

Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
 drivers/hid/hid-sensor-custom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index afffea894021..d7bdbae96b50 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -1042,9 +1042,9 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
 	}
 
 	hid_sensor_custom_dev_if_remove(sensor_inst);
-	hid_sensor_custom_remove_attributes(sensor_inst);
 	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
 			   &enable_sensor_attr_group);
+	hid_sensor_custom_remove_attributes(sensor_inst);
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
 }
 
-- 
2.25.1

[PATCH v2 2/2] HID: sensor: custom: Fix field sysfs group cleanup on failure

[Haoxiang Li]

hid_sensor_custom_add_attributes() creates one sysfs group for each
custom sensor field. If sysfs_create_group() fails after some groups
have already been created, the function returns the error without
removing the previously created groups. Add a local unwind path to
remove the groups that were already created.

Create the field attributes before exposing enable_sensor, so the
failure path can free sensor_inst->fields without leaving enable_sensor
able to access power_state or report_state pointers into that array.

Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
 drivers/hid/hid-sensor-custom.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index d7bdbae96b50..ea98088e5112 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -609,7 +609,7 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
 					 &sensor_inst->fields[i].
 					 hid_custom_attribute_group);
 		if (ret)
-			break;
+			goto err_remove_groups;
 
 		/* For power or report field store indexes */
 		if (sensor_inst->fields[i].attribute.attrib_id ==
@@ -621,6 +621,13 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
 	}
 
 	return ret;
+
+err_remove_groups:
+	while (--i >= 0)
+		sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
+				   &sensor_inst->fields[i].hid_custom_attribute_group);
+	kfree(sensor_inst->fields);
+	return ret;
 }
 
 static void hid_sensor_custom_remove_attributes(struct hid_sensor_custom *
@@ -1005,26 +1012,26 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
-				 &enable_sensor_attr_group);
+	ret = hid_sensor_custom_add_attributes(sensor_inst);
 	if (ret)
 		goto err_remove_callback;
 
-	ret = hid_sensor_custom_add_attributes(sensor_inst);
+	ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
+				 &enable_sensor_attr_group);
 	if (ret)
-		goto err_remove_group;
+		goto err_remove_attributes;
 
 	ret = hid_sensor_custom_dev_if_add(sensor_inst);
 	if (ret)
-		goto err_remove_attributes;
+		goto err_remove_group;
 
 	return 0;
 
-err_remove_attributes:
-	hid_sensor_custom_remove_attributes(sensor_inst);
 err_remove_group:
 	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
 			   &enable_sensor_attr_group);
+err_remove_attributes:
+	hid_sensor_custom_remove_attributes(sensor_inst);
 err_remove_callback:
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
 
-- 
2.25.1

Re: [PATCH v2 1/2] HID: sensor: custom: Remove enable_sensor before freeing fields

[Jonathan Cameron]

On Thu,  2 Jul 2026 17:48:55 +0800
Haoxiang Li <haoxiang_li2024@163.com> wrote:

> enable_sensor_store() can call set_power_report_state(), which
> dereferences sensor_inst->power_state and sensor_inst->report_state.
> These pointers refer to entries in sensor_inst->fields.
> 
> hid_sensor_custom_remove() currently frees the field attributes before
> removing the enable_sensor sysfs attribute, leaving a window where a
> concurrent sysfs write can dereference freed memory.
> 
> Remove enable_sensor before freeing the field attributes.
> 
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>

If this is the UAF that Jiri called out in patch one, please
credit the bot with a Reported-by tag and link to that review.  Example:
https://lore.kernel.org/all/20260702183644.60827-1-adrian.hunter@intel.com/


> ---
>  drivers/hid/hid-sensor-custom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index afffea894021..d7bdbae96b50 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -1042,9 +1042,9 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
>  	}
>  
>  	hid_sensor_custom_dev_if_remove(sensor_inst);
> -	hid_sensor_custom_remove_attributes(sensor_inst);
>  	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
>  			   &enable_sensor_attr_group);
> +	hid_sensor_custom_remove_attributes(sensor_inst);

Given this is out of order with respect to reversing what happens in probe,
please add a comment to say why (and ensure no one fixes it back to
the original order!)

It may be that a reorder in probe is needed as well to bring things into
balance and ensure that fields is available when
that enable_sensor_attr_group is registered but I haven't analysed it closely.


>  	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  }
>  

Re: [PATCH v2 2/2] HID: sensor: custom: Fix field sysfs group cleanup on failure

[Jonathan Cameron]

On Thu,  2 Jul 2026 17:48:56 +0800
Haoxiang Li <haoxiang_li2024@163.com> wrote:

> hid_sensor_custom_add_attributes() creates one sysfs group for each
> custom sensor field. If sysfs_create_group() fails after some groups
> have already been created, the function returns the error without
> removing the previously created groups. Add a local unwind path to
> remove the groups that were already created.
> 
> Create the field attributes before exposing enable_sensor, so the
> failure path can free sensor_inst->fields without leaving enable_sensor
> able to access power_state or report_state pointers into that array.
> 
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Ah. This has the reorder necessary to resolve the issue I called out
in previous patch (I think).  They can't be done independently.
Can you move the registration reorder to the previous patch then
only do the missing cleanup in this one?

> ---
>  drivers/hid/hid-sensor-custom.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index d7bdbae96b50..ea98088e5112 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -609,7 +609,7 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
>  					 &sensor_inst->fields[i].
>  					 hid_custom_attribute_group);
>  		if (ret)
> -			break;
> +			goto err_remove_groups;
>  
>  		/* For power or report field store indexes */
>  		if (sensor_inst->fields[i].attribute.attrib_id ==
> @@ -621,6 +621,13 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
>  	}
>  
>  	return ret;
> +
> +err_remove_groups:
> +	while (--i >= 0)
> +		sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> +				   &sensor_inst->fields[i].hid_custom_attribute_group);
> +	kfree(sensor_inst->fields);
> +	return ret;
>  }
>  
>  static void hid_sensor_custom_remove_attributes(struct hid_sensor_custom *
> @@ -1005,26 +1012,26 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
> -				 &enable_sensor_attr_group);
> +	ret = hid_sensor_custom_add_attributes(sensor_inst);

I think this brings things back into order wrt to remove.
This probably needs to be in the previous patch.

>  	if (ret)
>  		goto err_remove_callback;
>  
> -	ret = hid_sensor_custom_add_attributes(sensor_inst);
> +	ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
> +				 &enable_sensor_attr_group);
>  	if (ret)
> -		goto err_remove_group;
> +		goto err_remove_attributes;
>  
>  	ret = hid_sensor_custom_dev_if_add(sensor_inst);
>  	if (ret)
> -		goto err_remove_attributes;
> +		goto err_remove_group;
>  
>  	return 0;
>  
> -err_remove_attributes:
> -	hid_sensor_custom_remove_attributes(sensor_inst);
>  err_remove_group:
>  	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
>  			   &enable_sensor_attr_group);
> +err_remove_attributes:
> +	hid_sensor_custom_remove_attributes(sensor_inst);
>  err_remove_callback:
>  	sensor_hub_remove_callback(hsdev, hsdev->usage);
>