Re: [PATCH v2 4/7] HID: sensor: custom: Fix state lifetime during removal

[Jonathan Cameron]

On Wed,  4 Nov 2026 12:14:00 -0800
Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:

> Open misc device files can outlive platform-device removal. Since the
> devm-allocated state can be freed while a reader is queued on its wait
> queue, subsequent file operations may access freed memory.
> 
> Allocate the state independently of devres and give every open
> file a reference to keep the FIFO and state alive after removal.
> 
> On removal, mark the device as disconnected and wake readers; make reads
> return -ENODEV and polls report hangup and error. Free the FIFO and state
> when the final reference is released. Unregister the sensor hub callback
> before tearing down the interface so in-flight raw-event processing
> finishes before the sensor state can be freed.
> 
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
> Reported-by: Семён Марьясин <simeon@xxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/CAM2dg7ewfVJSP4rFyhkCB1Rmd1+X56sAg=eUdbPthG2xrLqwQQ@xxxxxxxxxxxxxx/
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>

Looks like fairly standard fix for this style of lifetime issue.

One trivial comment inline.


>  
>  /*
> @@ -978,8 +1004,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  	int ret;
>  	const struct hid_sensor_custom_match *match;
>  
> -	sensor_inst = devm_kzalloc(&pdev->dev, sizeof(*sensor_inst),
> -				   GFP_KERNEL);
> +	sensor_inst = kzalloc_obj(*sensor_inst);
>  	if (!sensor_inst)
>  		return -ENOMEM;
>  
> @@ -989,6 +1014,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  	sensor_inst->hsdev = hsdev;
>  	sensor_inst->pdev = pdev;
>  	mutex_init(&sensor_inst->mutex);
> +	kref_init(&sensor_inst->ref);
>  	platform_set_drvdata(pdev, sensor_inst);
>  
>  	ret = hid_sensor_custom_get_known(hsdev, &match);
> @@ -1000,7 +1026,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  		if (ret) {
>  			dev_err(&pdev->dev,
>  				"register_platform_device failed\n");
> -			return ret;
> +			goto err_free;
>  		}
>  
>  		return 0;
> @@ -1010,7 +1036,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  					   &sensor_inst->callbacks);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		return ret;
> +		goto err_free;

Trivial but given it might free I'd all that label err_put

>  	}
>  
>  	ret = hid_sensor_custom_add_attributes(sensor_inst);
> @@ -1035,6 +1061,8 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
>  	hid_sensor_custom_remove_attributes(sensor_inst);
>  err_remove_callback:
>  	sensor_hub_remove_callback(hsdev, hsdev->usage);
> +err_free:
> +	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
>  
>  	return ret;
>  }
> @@ -1046,9 +1074,10 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
>  
>  	if (sensor_inst->custom_pdev) {
>  		platform_device_unregister(sensor_inst->custom_pdev);
> -		return;
> +		goto out;
>  	}
>  
> +	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  	hid_sensor_custom_dev_if_remove(sensor_inst);
>  	/* Remove enable_sensor first as it uses fields via power_state/report_state. */
>  	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> @@ -1061,7 +1090,8 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
>  	}
>  	mutex_unlock(&sensor_inst->mutex);
>  	hid_sensor_custom_remove_attributes(sensor_inst);
> -	sensor_hub_remove_callback(hsdev, hsdev->usage);
> +out:
> +	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
>  }
>  
>  static const struct platform_device_id hid_sensor_custom_ids[] = {