Skip to content

Commit 905d754

Browse files
ndreysbentiss
authored andcommitted
HID: logitech-hidpp: rework device validation
G920 device only advertises REPORT_ID_HIDPP_LONG and REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying for REPORT_ID_HIDPP_SHORT with optional=false will always fail and prevent G920 to be recognized as a valid HID++ device. To fix this and improve some other aspects, modify hidpp_validate_device() as follows: - Inline the code of hidpp_validate_report() to simplify distingushing between non-present and invalid report descriptors - Drop the check for id >= HID_MAX_IDS || id < 0 since all of our IDs are static and known to satisfy that at compile time - Change the algorithms to check all possible report types (including very long report) and deem the device as a valid HID++ device if it supports at least one - Treat invalid report length as a hard stop for the validation algorithm, meaning that if any of the supported reports has invalid length we assume the worst and treat the device as a generic HID device. - Fold initialization of hidpp->very_long_report_length into hidpp_validate_device() since it already fetches very long report length and validates its value Fixes: fe3ee1e ("HID: logitech-hidpp: allow non HID++ devices to be handled by this module") Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191 Reported-by: Sam Bazely <[email protected]> Signed-off-by: Andrey Smirnov <[email protected]> Cc: Jiri Kosina <[email protected]> Cc: Benjamin Tissoires <[email protected]> Cc: Henrik Rydberg <[email protected]> Cc: Pierre-Loup A. Griffais <[email protected]> Cc: Austin Palmer <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] # 5.2+ Signed-off-by: Benjamin Tissoires <[email protected]>
1 parent abdd3d0 commit 905d754

File tree

1 file changed

+30
-24
lines changed

1 file changed

+30
-24
lines changed

drivers/hid/hid-logitech-hidpp.c

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id)
34983498
return report->field[0]->report_count + 1;
34993499
}
35003500

3501-
static bool hidpp_validate_report(struct hid_device *hdev, int id,
3502-
int expected_length, bool optional)
3501+
static bool hidpp_validate_device(struct hid_device *hdev)
35033502
{
3504-
int report_length;
3503+
struct hidpp_device *hidpp = hid_get_drvdata(hdev);
3504+
int id, report_length, supported_reports = 0;
3505+
3506+
id = REPORT_ID_HIDPP_SHORT;
3507+
report_length = hidpp_get_report_length(hdev, id);
3508+
if (report_length) {
3509+
if (report_length < HIDPP_REPORT_SHORT_LENGTH)
3510+
goto bad_device;
35053511

3506-
if (id >= HID_MAX_IDS || id < 0) {
3507-
hid_err(hdev, "invalid HID report id %u\n", id);
3508-
return false;
3512+
supported_reports++;
35093513
}
35103514

3515+
id = REPORT_ID_HIDPP_LONG;
35113516
report_length = hidpp_get_report_length(hdev, id);
3512-
if (!report_length)
3513-
return optional;
3517+
if (report_length) {
3518+
if (report_length < HIDPP_REPORT_LONG_LENGTH)
3519+
goto bad_device;
35143520

3515-
if (report_length < expected_length) {
3516-
hid_warn(hdev, "not enough values in hidpp report %d\n", id);
3517-
return false;
3521+
supported_reports++;
35183522
}
35193523

3520-
return true;
3521-
}
3524+
id = REPORT_ID_HIDPP_VERY_LONG;
3525+
report_length = hidpp_get_report_length(hdev, id);
3526+
if (report_length) {
3527+
if (report_length < HIDPP_REPORT_LONG_LENGTH ||
3528+
report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
3529+
goto bad_device;
35223530

3523-
static bool hidpp_validate_device(struct hid_device *hdev)
3524-
{
3525-
return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
3526-
HIDPP_REPORT_SHORT_LENGTH, false) &&
3527-
hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
3528-
HIDPP_REPORT_LONG_LENGTH, true);
3531+
supported_reports++;
3532+
hidpp->very_long_report_length = report_length;
3533+
}
3534+
3535+
return supported_reports;
3536+
3537+
bad_device:
3538+
hid_warn(hdev, "not enough values in hidpp report %d\n", id);
3539+
return false;
35293540
}
35303541

35313542
static bool hidpp_application_equals(struct hid_device *hdev,
@@ -3572,11 +3583,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
35723583
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
35733584
}
35743585

3575-
hidpp->very_long_report_length =
3576-
hidpp_get_report_length(hdev, REPORT_ID_HIDPP_VERY_LONG);
3577-
if (hidpp->very_long_report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
3578-
hidpp->very_long_report_length = HIDPP_REPORT_VERY_LONG_MAX_LENGTH;
3579-
35803586
if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
35813587
hidpp->quirks |= HIDPP_QUIRK_UNIFYING;
35823588

0 commit comments

Comments
 (0)