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: fe3ee1ec007b ("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 <sambazley(a)fastmail.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov(a)gmail.com>
Cc: Jiri Kosina <jikos(a)kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires(a)redhat.com>
Cc: Henrik Rydberg <rydberg(a)bitmath.org>
Cc: Pierre-Loup A. Griffais <pgriffais(a)valvesoftware.com>
Cc: Austin Palmer <austinp(a)valvesoftware.com>
Cc: linux-input(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
Cc: stable(a)vger.kernel.org # 5.2+
---
drivers/hid/hid-logitech-hidpp.c | 54 ++++++++++++++++++--------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 85911586b3b6..6e669eb7dc69 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id)
return report->field[0]->report_count + 1;
}
-static bool hidpp_validate_report(struct hid_device *hdev, int id,
- int expected_length, bool optional)
+static bool hidpp_validate_device(struct hid_device *hdev)
{
- int report_length;
+ struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+ int id, report_length, supported_reports = 0;
+
+ id = REPORT_ID_HIDPP_SHORT;
+ report_length = hidpp_get_report_length(hdev, id);
+ if (report_length) {
+ if (report_length < HIDPP_REPORT_SHORT_LENGTH)
+ goto bad_device;
- if (id >= HID_MAX_IDS || id < 0) {
- hid_err(hdev, "invalid HID report id %u\n", id);
- return false;
+ supported_reports++;
}
+ id = REPORT_ID_HIDPP_LONG;
report_length = hidpp_get_report_length(hdev, id);
- if (!report_length)
- return optional;
+ if (report_length) {
+ if (report_length < HIDPP_REPORT_LONG_LENGTH)
+ goto bad_device;
- if (report_length < expected_length) {
- hid_warn(hdev, "not enough values in hidpp report %d\n", id);
- return false;
+ supported_reports++;
}
- return true;
-}
+ id = REPORT_ID_HIDPP_VERY_LONG;
+ report_length = hidpp_get_report_length(hdev, id);
+ if (report_length) {
+ if (report_length < HIDPP_REPORT_LONG_LENGTH ||
+ report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
+ goto bad_device;
-static bool hidpp_validate_device(struct hid_device *hdev)
-{
- return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
- HIDPP_REPORT_SHORT_LENGTH, false) &&
- hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
- HIDPP_REPORT_LONG_LENGTH, true);
+ supported_reports++;
+ hidpp->very_long_report_length = report_length;
+ }
+
+ return supported_reports;
+
+bad_device:
+ hid_warn(hdev, "not enough values in hidpp report %d\n", id);
+ return false;
}
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)
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
}
- hidpp->very_long_report_length =
- hidpp_get_report_length(hdev, REPORT_ID_HIDPP_VERY_LONG);
- if (hidpp->very_long_report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
- hidpp->very_long_report_length = HIDPP_REPORT_VERY_LONG_MAX_LENGTH;
-
if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
hidpp->quirks |= HIDPP_QUIRK_UNIFYING;
--
2.21.0
On Thu, Oct 17, 2019 at 7:31 AM Sasha Levin <sashal(a)kernel.org> wrote:
>
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: ff21a635dd1a9 HID: logitech-hidpp: Force feedback support for the Logitech G920.
>
> The bot has tested the following trees: v5.3.6, v4.19.79, v4.14.149, v4.9.196.
>
> v5.3.6: Build OK!
> v4.19.79: Failed to apply! Possible dependencies:
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
>
> v4.14.149: Failed to apply! Possible dependencies:
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
>
> v4.9.196: Failed to apply! Possible dependencies:
> 6bd4e65d521f9 ("HID: logitech-hidpp: remove HIDPP_QUIRK_CONNECT_EVENTS")
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
> a4bf6153b3177 ("HID: logitech-hidpp: add a sysfs file to tell we support power_supply")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
Looks like I'd have to mark this one as 5.2+ as well. Please disregard
this series in favor of v3 that will be sent out shortly.
Thanks,
Andrey Smirnov
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.3.6, v4.19.79, v4.14.149, v4.9.196, v4.4.196.
v5.3.6: Failed to apply! Possible dependencies:
18d9be1a970c3 ("io_uring: add io_queue_async_work() helper")
4fe2c963154c3 ("io_uring: add support for link with drain")
5262f567987d3 ("io_uring: IORING_OP_TIMEOUT support")
75b28affdd6ae ("io_uring: allocate the two rings together")
c576666863b78 ("io_uring: optimize submit_and_wait API")
v4.19.79: Failed to apply! Possible dependencies:
2b188cc1bb857 ("Add io_uring IO interface")
31b515106428b ("io_uring: allow workqueue item to handle multiple buffered requests")
4e21565b7fd4d ("asm-generic: add kexec_file_load system call to unistd.h")
5262f567987d3 ("io_uring: IORING_OP_TIMEOUT support")
6b06314c47e14 ("io_uring: add file set registration")
9a56a2323dbbd ("io_uring: use fget/fput_many() for file references")
c992fe2925d77 ("io_uring: add fsync support")
d530a402a114e ("io_uring: add prepped flag")
de0617e467171 ("io_uring: add support for marking commands as draining")
def596e9557c9 ("io_uring: support for IO polling")
edafccee56ff3 ("io_uring: add support for pre-mapped user IO buffers")
v4.14.149: Failed to apply! Possible dependencies:
05c17cedf85ba ("x86: Wire up restartable sequence system call")
1bd21c6c21e84 ("syscalls/core: Introduce CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y")
2b188cc1bb857 ("Add io_uring IO interface")
3c1c456f9b96c ("syscalls: sort syscall prototypes in include/linux/syscalls.h")
4ddb45db30851 ("x86/syscalls: Use COMPAT_SYSCALL_DEFINEx() macros for x86-only compat syscalls")
5262f567987d3 ("io_uring: IORING_OP_TIMEOUT support")
5ac9efa3c50d7 ("syscalls/core, syscalls/x86: Clean up compat syscall stub naming convention")
66f4e88cc69da ("x86/ioport: add ksys_ioperm() helper; remove in-kernel calls to sys_ioperm()")
7a074e96dee62 ("aio: implement io_pgetevents")
7c2178c1ff482 ("x86/syscalls: Use proper syscall definition for sys_ioperm()")
ab0d1e85bfd0c ("fs/quota: use COMPAT_SYSCALL_DEFINE for sys32_quotactl()")
af52201d99162 ("x86/entry: Do not special-case clone(2) in compat entry")
b411991e0ca88 ("x86/syscalls/32: Simplify $entry == $compat entries")
b51d3cdf44d5c ("x86: remove compat_sys_x86_waitpid()")
d53238cd51a80 ("kernel: open-code sys_rt_sigpending() in sys_sigpending()")
de0617e467171 ("io_uring: add support for marking commands as draining")
dfe64506c01e5 ("x86/syscalls: Don't pointlessly reload the system call number")
e145242ea0df6 ("syscalls/core, syscalls/x86: Clean up syscall stub naming convention")
ebeb8c82ffaf9 ("syscalls/x86: Use 'struct pt_regs' based syscall calling for IA32_EMULATION and x32")
fa697140f9a20 ("syscalls/x86: Use 'struct pt_regs' based syscall calling convention for 64-bit syscalls")
v4.9.196: Failed to apply! Possible dependencies:
05c17cedf85ba ("x86: Wire up restartable sequence system call")
2611dc1939569 ("Remove compat_sys_getdents64()")
2b188cc1bb857 ("Add io_uring IO interface")
3a404842547c9 ("x86/entry: define _TIF_ALLWORK_MASK flags explicitly")
499934898fcd1 ("x86/entry/64: Use ENTRY() instead of ALIGN+GLOBAL for stub32_clone()")
4ddb45db30851 ("x86/syscalls: Use COMPAT_SYSCALL_DEFINEx() macros for x86-only compat syscalls")
5262f567987d3 ("io_uring: IORING_OP_TIMEOUT support")
5ac9efa3c50d7 ("syscalls/core, syscalls/x86: Clean up compat syscall stub naming convention")
5ea0727b163cb ("x86/syscalls: Check address limit on user-mode return")
66f4e88cc69da ("x86/ioport: add ksys_ioperm() helper; remove in-kernel calls to sys_ioperm()")
7a074e96dee62 ("aio: implement io_pgetevents")
7c2178c1ff482 ("x86/syscalls: Use proper syscall definition for sys_ioperm()")
a528d35e8bfcc ("statx: Add a system call to make enhanced file info available")
ab0d1e85bfd0c ("fs/quota: use COMPAT_SYSCALL_DEFINE for sys32_quotactl()")
af52201d99162 ("x86/entry: Do not special-case clone(2) in compat entry")
afb94c9e0b413 ("livepatch/x86: add TIF_PATCH_PENDING thread flag")
b411991e0ca88 ("x86/syscalls/32: Simplify $entry == $compat entries")
b51d3cdf44d5c ("x86: remove compat_sys_x86_waitpid()")
de0617e467171 ("io_uring: add support for marking commands as draining")
dfe64506c01e5 ("x86/syscalls: Don't pointlessly reload the system call number")
e145242ea0df6 ("syscalls/core, syscalls/x86: Clean up syscall stub naming convention")
ebeb8c82ffaf9 ("syscalls/x86: Use 'struct pt_regs' based syscall calling for IA32_EMULATION and x32")
fa697140f9a20 ("syscalls/x86: Use 'struct pt_regs' based syscall calling convention for 64-bit syscalls")
v4.4.196: Failed to apply! Possible dependencies:
05c17cedf85ba ("x86: Wire up restartable sequence system call")
1e423bff959e4 ("x86/entry/64: Migrate the 64-bit syscall slow path to C")
2b188cc1bb857 ("Add io_uring IO interface")
302f5b260c322 ("x86/entry/64: Always run ptregs-using syscalls on the slow path")
3e65654e3db6d ("x86/syscalls: Move compat syscall entry handling into syscalltbl.sh")
46eabf06c04a6 ("x86/entry/64: Call all native slow-path syscalls with full pt-regs")
5262f567987d3 ("io_uring: IORING_OP_TIMEOUT support")
5ac9efa3c50d7 ("syscalls/core, syscalls/x86: Clean up compat syscall stub naming convention")
7a074e96dee62 ("aio: implement io_pgetevents")
95d97adb2bb85 ("x86/signal: Cleanup get_nr_restart_syscall()")
97245d00585d8 ("x86/entry: Get rid of pt_regs_to_thread_info()")
abfb9498ee132 ("x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()")
c87a85177e7a7 ("x86/entry: Get rid of two-phase syscall entry work")
cfcbadb49dabb ("x86/syscalls: Add syscall entry qualifiers")
de0617e467171 ("io_uring: add support for marking commands as draining")
dfe64506c01e5 ("x86/syscalls: Don't pointlessly reload the system call number")
e145242ea0df6 ("syscalls/core, syscalls/x86: Clean up syscall stub naming convention")
ebeb8c82ffaf9 ("syscalls/x86: Use 'struct pt_regs' based syscall calling for IA32_EMULATION and x32")
fa697140f9a20 ("syscalls/x86: Use 'struct pt_regs' based syscall calling convention for 64-bit syscalls")
fba324744bfd2 ("x86/syscalls: Refactor syscalltbl.sh")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks,
Sasha