| Message ID | 20250401063157.19655-1-bsdhenrymartin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] HID: uclogic: Add NULL check in uclogic_input_configured | expand |
> devm_kasprintf() return NULL if memory allocation fails. Currently, … call? failed? > Add NULL check after devm_kasprintf() to prevent this issue. Do you propose to improve this function implementation a bit more? … > --- > V1 -> V2: > Simplify the handing of the condition "suffix" with if/else suggested by … * I imagine that there is a need to offer such adjustments in separate update steps. https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14#n81 * Can another tag become more helpful? https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14#n598 > The current implementation (directly returning -ENOMEM) is reasonable because: … How much will such information influence the refinement of the change description? How do you think about to append parentheses to a function name also in the summary phrase? Regards, Markus
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c index d8008933c052..83625ec6a55a 100644 --- a/drivers/hid/hid-uclogic-core.c +++ b/drivers/hid/hid-uclogic-core.c @@ -118,35 +118,29 @@ static int uclogic_input_configured(struct hid_device *hdev, } } - if (!suffix) { + if (!suffix && hi->report->maxfield > 0) { field = hi->report->field[0]; - switch (field->application) { - case HID_GD_KEYBOARD: + if (field->application == HID_GD_KEYBOARD) suffix = "Keyboard"; - break; - case HID_GD_MOUSE: + else if (field->application == HID_GD_MOUSE) suffix = "Mouse"; - break; - case HID_GD_KEYPAD: + else if (field->application == HID_GD_KEYPAD) suffix = "Pad"; - break; - case HID_DG_PEN: - case HID_DG_DIGITIZER: + else if (field->application == HID_DG_PEN || field->application == HID_DG_DIGITIZER) suffix = "Pen"; - break; - case HID_CP_CONSUMER_CONTROL: + else if (field->application == HID_CP_CONSUMER_CONTROL) suffix = "Consumer Control"; - break; - case HID_GD_SYSTEM_CONTROL: + else if (field->application == HID_GD_SYSTEM_CONTROL) suffix = "System Control"; - break; - } } - if (suffix) + if (suffix) { hi->input->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s %s", hdev->name, suffix); + if (!hi->input->name) + return -ENOMEM; + } return 0; }
devm_kasprintf() returns NULL when memory allocation fails. Currently, uclogic_input_configured() does not check for this case, which results in a NULL pointer dereference. Add NULL check after devm_kasprintf() to prevent this issue. Fixes: dd613a4e45f8 ("HID: uclogic: Correct devm device reference for hidinput input_dev name") Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com> --- V1 -> V2: Simplify the handing of the condition "suffix" with if/else suggested by Markus. The current implementation (directly returning -ENOMEM) is reasonable because: 1. pen_input is just a cached pointer - no resources are allocated 2. evbit modification is a software-only configuration (hardware-independent) 3. No critical state needs rollback on memory allocation failure drivers/hid/hid-uclogic-core.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)