Re: [PATCH] Added a check for NULL pointer in hid_add_device
From: Benjamin Tissoires <hidden>
Date: 2013-06-26 14:04:39
Also in:
lkml
Hi Michael, On 06/25/2013 07:51 PM, Michael Banken wrote:
This check greatly simplifies creating a dummy hid device.
Could you elaborate a little here? If you want to create a hid device from the user space, I encourage you to use uhid, which is available since v3.6.
quoted hunk
With this check in place a hid dummy can be created simply by allocating and adding the device. This used to be possible in earlier Kernel versions. Signed-off-by: Michael Banken <redacted> Signed-off-by: Lorenz Haspel <redacted> --- drivers/hid/hid-core.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 0951a9a..88c573e 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c@@ -2289,24 +2289,27 @@ int hid_add_device(struct hid_device *hdev) if (hid_ignore(hdev)) return -ENODEV; - /* - * Read the device report descriptor once and use as template - * for the driver-specific modifications. - */ - ret = hdev->ll_driver->parse(hdev); - if (ret) - return ret; - if (!hdev->dev_rdesc) - return -ENODEV; - - /* - * Scan generic devices for group information - */ - if (hid_ignore_special_drivers || - !hid_match_id(hdev, hid_have_special_driver)) { - ret = hid_scan_report(hdev); + if (hdev->ll_driver != NULL) {
I am concerned by the fact that *every* hid specific driver will have to call hid_hw_start and hid_hw_stop at some point. These 2 functions are not protected against a null pointer in hdev->ll_driver, so allowing here a null pointer will introduce a kernel oops later. I think it would be safer to have: if (!hdev->ll_driver) return -ENODEV; But if I understood correctly, this is not your point. So definitively, I need your usage scenarios of such hid "dummy" device. Cheers, Benjamin
+ /*
+ * Read the device report descriptor once and use as template
+ * for the driver-specific modifications.
+ */
+ ret = hdev->ll_driver->parse(hdev);
if (ret)
- hid_warn(hdev, "bad device descriptor (%d)\n", ret);
+ return ret;
+ if (!hdev->dev_rdesc)
+ return -ENODEV;
+
+ /*
+ * Scan generic devices for group information
+ */
+ if (hid_ignore_special_drivers ||
+ !hid_match_id(hdev, hid_have_special_driver)) {
+ ret = hid_scan_report(hdev);
+ if (ret)
+ hid_warn(hdev,
+ "bad device descriptor (%d)\n", ret);
+ }
}
/* XXX hack, any other cleaner solution after the driver core