Re: [PATCH v2] hwmon: (corsair-cpro) Create debugfs entries after hwmon registration

[Guenter Roeck]

On Mon, Aug 31, 2026 at 09:45:09AM +0800, Linmao Li wrote:
> ccp_debugfs_init() registers debugfs files whose private data is the devm
> allocated ccp.  It runs before hwmon_device_register_with_info(), so when
> that registration fails, ccp_probe() returns with the files still in
> place.  The HID core then frees ccp, and ccp_remove() is not called for a
> failed probe, so nothing removes them later either.  Reading one of the
> files dereferences the freed pointer.
> 
> Create the debugfs entries only after the hwmon device has been
> registered, so no failing path can leave them behind.
> 
> The two version queries stay where they are.  They send USB commands
> without holding ccp->mutex, which is only safe as long as nothing else
> can call send_usb_cmd(); once the hwmon device is registered its
> callbacks can do so concurrently.  Only the debugfs creation moves, and
> it is told which queries succeeded.
> 
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-hwmon/20260708031612.BD7E61F000E9@xxxxxxxxxxxxxxx/
> Suggested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Fixes: 5997eb60f896 ("hwmon: (corsair-cpro) Add firmware and bootloader information")
> Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>

Applied.

Thanks,
Guenter

> ---
> 
> Notes (v2changelog):
>     v2: Create the debugfs entries after hwmon registration instead of
>         removing them on the error path, as suggested by Guenter Roeck.  The
>         version queries stay before the registration.
>     Link: https://lore.kernel.org/linux-hwmon/20260828061949.3151191-1-lilinmao@xxxxxxxxxx/
> 
>  drivers/hwmon/corsair-cpro.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
> 
> diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
> index 8354a002f4c5..56de0fe0f544 100644
> --- a/drivers/hwmon/corsair-cpro.c
> +++ b/drivers/hwmon/corsair-cpro.c
> @@ -566,21 +566,18 @@ static int bootloader_show(struct seq_file *seqf, void *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(bootloader);
>  
> -static void ccp_debugfs_init(struct ccp_device *ccp)
> +static void ccp_debugfs_init(struct ccp_device *ccp, bool fw_valid, bool bl_valid)
>  {
>  	char name[32];
> -	int ret;
>  
>  	scnprintf(name, sizeof(name), "corsaircpro-%s", dev_name(&ccp->hdev->dev));
>  	ccp->debugfs = debugfs_create_dir(name, NULL);
>  
> -	ret = get_fw_version(ccp);
> -	if (!ret)
> +	if (fw_valid)
>  		debugfs_create_file("firmware_version", 0444,
>  				    ccp->debugfs, ccp, &firmware_fops);
>  
> -	ret = get_bl_version(ccp);
> -	if (!ret)
> +	if (bl_valid)
>  		debugfs_create_file("bootloader_version", 0444,
>  				    ccp->debugfs, ccp, &bootloader_fops);
>  }
> @@ -588,6 +585,7 @@ static void ccp_debugfs_init(struct ccp_device *ccp)
>  static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>  	struct ccp_device *ccp;
> +	bool fw_valid, bl_valid;
>  	int ret;
>  
>  	ccp = devm_kzalloc(&hdev->dev, sizeof(*ccp), GFP_KERNEL);
> @@ -632,7 +630,13 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (ret)
>  		goto out_hw_close;
>  
> -	ccp_debugfs_init(ccp);
> +	/*
> +	 * Query the versions before registering the hwmon device: they send
> +	 * USB commands without holding ccp->mutex, which is only safe while
> +	 * nothing else can call send_usb_cmd().
> +	 */
> +	fw_valid = !get_fw_version(ccp);
> +	bl_valid = !get_bl_version(ccp);
>  
>  	ccp->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsaircpro",
>  							 ccp, &ccp_chip_info, NULL);
> @@ -641,6 +645,8 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		goto out_hw_close;
>  	}
>  
> +	ccp_debugfs_init(ccp, fw_valid, bl_valid);
> +
>  	return 0;
>  
>  out_hw_close: