Re: [PATCH] hwmon: (corsair-cpro) Remove debugfs entries when probe fails

[Linmao Li]

在 2026/8/28 21:14, Guenter Roeck 写道:
> On 8/27/26 23:19, Linmao Li wrote:
> > ccp_debugfs_init() registers debugfs files whose private data is the 
> > devm
> > allocated ccp.  If hwmon_device_register_with_info() fails right 
> > after it,
> > ccp_probe() returns without removing them: the HID core then frees ccp,
> > and ccp_remove() is not called for a failed probe, so the files stay
> > behind.  Reading one of them dereferences the freed pointer.
> >
> > Remove the debugfs entries on that error path. 
> > debugfs_remove_recursive()
> > waits for readers already inside the show callbacks, so ccp is no longer
> > reachable through debugfs by the time probe returns.
> >
> > Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> > Closes: 
> > https://lore.kernel.org/linux-hwmon/20260708031612.BD7E61F000E9@xxxxxxxxxxxxxxx/
> > Fixes: 5997eb60f896 ("hwmon: (corsair-cpro) Add firmware and 
> > bootloader information")
> > Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> > ---
> >   drivers/hwmon/corsair-cpro.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
> > index 8354a002f4c5..30fe5d8e724c 100644
> > --- a/drivers/hwmon/corsair-cpro.c
> > +++ b/drivers/hwmon/corsair-cpro.c
> > @@ -638,11 +638,13 @@ static int ccp_probe(struct hid_device *hdev, 
> > const struct hid_device_id *id)
> >                                ccp, &ccp_chip_info, NULL);
> >       if (IS_ERR(ccp->hwmon_dev)) {
> >           ret = PTR_ERR(ccp->hwmon_dev);
> > -        goto out_hw_close;
> > +        goto out_debugfs_remove;
> >       }
>
> Any reason to not move the call to ccp_debugfs_init() instead ?
ccp_debugfs_init() also calls get_fw_version() and get_bl_version(), which
send USB commands without taking ccp->mutex. That is fine where the call
sits now, because nothing else calls send_usb_cmd() until the hwmon device
is registered. Moving it unchanged past the registration would let the
hwmon callbacks race with those two reads on ccp->cmd_buffer, ccp->buffer
and wait_input_report.

I will split the version queries from the debugfs creation in v2: the
queries stay where they are, and the debugfs entries are created only
after hwmon_device_register_with_info() has succeeded, which removes the
error path altogether.

Thanks,
Linmao
> Thanks,
> Guenter
>
> >       return 0;
> >   +out_debugfs_remove:
> > +    debugfs_remove_recursive(ccp->debugfs);
> >   out_hw_close:
> >       hid_hw_close(hdev);
> >       hid_device_io_stop(hdev);
> >
> > base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f