[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit ba9de350509504fb748837b71e23d7e84c83d93c ]
Calling get_wireless_feature_index() from probe() causes
the wireless_feature_index to only get set for unifying devices which
are already connected at probe() time. It does not get set for devices
which connect later.
Fix this by moving get_wireless_feature_index() to hidpp_connect_event(),
this does not make a difference for devices connected at probe() since
probe() will queue the hidpp_connect_event() for those at probe time.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
And by bentiss:
Logitech Touchpad T650 (unifying)
Logitech Touchpad T651 (bluetooth)
Logitech MX Master 3B (BLE)
Logitech G403 (plain USB / Gaming receiver)
Fixes: 0da0a63b7cba ("HID: logitech-hidpp: Support WirelessDeviceStatus connect events")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231010102029.111003-4-hdegoede@redhat.com
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 55bf70362ffc4ddd7c8745e2fe880edac00e4aff ]
Commit 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
makes hidpp_probe() first call hid_hw_start(hdev, 0) to allow IO
without connecting any hid subdrivers (hid-input, hidraw).
This is done to allow to retrieve the device's name and serial number
and store these in hdev->name and hdev->uniq.
Then later on IO was stopped and started again with hid_hw_start(hdev,
HID_CONNECT_DEFAULT) connecting hid-input and hidraw after the name
and serial number have been setup.
Commit 498ba2069035 ("HID: logitech-hidpp: Don't restart communication
if not necessary") changed the probe() code to only do the start with
a 0 connect-mask + restart later for unifying devices.
But for non unifying devices hdev->name and hdev->uniq are updated too.
So this change re-introduces the problem for which the start with
a 0 connect-mask + restart later behavior was introduced.
The previous patch in this series changes the unifying path to instead of
restarting IO only call hid_connect() later. This avoids possible issues
with restarting IO seen on non unifying devices.
Revert the change to limit the restart behavior to unifying devices to
fix hdev->name changing after userspace facing devices have already been
registered.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
And by bentiss:
Logitech Touchpad T650 (unifying)
Logitech Touchpad T651 (bluetooth)
Logitech MX Master 3B (BLE)
Logitech G403 (plain USB / Gaming receiver)
Fixes: 498ba2069035 ("HID: logitech-hidpp: Don't restart communication if not necessary")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231010102029.111003-3-hdegoede@redhat.com
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 11ca0322a41920df2b462d2e45b0731e47ff475b ]
Restarting IO causes 2 problems:
1. Some devices do not like IO being restarted this was addressed in
commit 498ba2069035 ("HID: logitech-hidpp: Don't restart communication
if not necessary"), but that change has issues of its own and needs to
be reverted.
2. Restarting IO and specifically calling hid_device_io_stop() causes
received packets to be missed, which may cause connect-events to
get missed.
Restarting IO was introduced in commit 91cf9a98ae41 ("HID: logitech-hidpp:
make .probe usbhid capable") to allow to retrieve the device's name and
serial number and store these in hdev->name and hdev->uniq before
connecting any hid subdrivers (hid-input, hidraw) exporting this info
to userspace.
But this does not require restarting IO, this merely requires deferring
calling hid_connect(). Calling hid_hw_start() with a connect-mask of
0 makes it skip calling hid_connect(), so hidpp_probe() can simply call
hid_connect() later without needing to restart IO.
Remove the stop + restart of IO and instead just call hid_connect() later
to avoid the issues caused by restarting IO.
Now that IO is no longer stopped, hid_hw_close() must be called at the end
of probe() to balance the hid_hw_open() done at the beginning probe().
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
And by bentiss:
Logitech Touchpad T650 (unifying)
Logitech Touchpad T651 (bluetooth)
Logitech MX Master 3B (BLE)
Logitech G403 (plain USB / Gaming receiver)
Fixes: 498ba2069035 ("HID: logitech-hidpp: Don't restart communication if not necessary")
Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231010102029.111003-2-hdegoede@redhat.com
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit d83956c8855c6c2ed4bd16cec4a5083d63df17e4 ]
HIDPP_QUIRK_NO_HIDINPUT isn't used by any devices but still happens to
work as HIDPP_QUIRK_DELAYED_INIT is defined to the same value. Remove
HIDPP_QUIRK_NO_HIDINPUT and use HIDPP_QUIRK_DELAYED_INIT everywhere
instead.
Tested on a T650 which requires that quirk, and a number of unifying and
Bluetooth devices that don't.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
Link: https://lore.kernel.org/r/20230125121723.3122-2-hadess@hadess.net
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Stable-dep-of: 11ca0322a419 ("HID: logitech-hidpp: Don't restart IO, instead defer hid_connect() only")
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit cae253d6033da885e71c29c1591b22838a52de76 ]
Now that we're in 2022, and the majority of desktop environments can and
should support touchpad gestures through libinput, remove the legacy
module parameter that made it possible to use gestures implemented in
firmware.
This will eventually allow simplifying the driver's initialisation code.
This reverts commit 9188dbaed68a4b23dc96eba165265c08caa7dc2a.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Link: https://lore.kernel.org/r/20221220154345.474596-1-hadess@hadess.net
Stable-dep-of: 11ca0322a419 ("HID: logitech-hidpp: Don't restart IO, instead defer hid_connect() only")
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit dc3115e6c5d9863ec1a9ff1acf004ede93c34361 ]
Previously cp2112_gpio_irq_shutdown() always cancelled the
gpio_poll_worker, even if other IRQs were still active, and did not set
the gpio_poll flag to false. This resulted in any call to _shutdown()
resulting in interrupts no longer functioning on the chip until a
_remove() occurred (a.e. the cp2112 is unplugged or system rebooted).
Only cancel polling if all IRQs are disabled/masked, and correctly set
the gpio_poll flag, allowing polling to restart when an interrupt is
next enabled.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
Fixes: 13de9cca514e ("HID: cp2112: add IRQ chip handling")
Link: https://lore.kernel.org/r/20231011182317.1053344-1-danny.kaehn@plexus.com
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ Upstream commit e3c2d2d144c082dd71596953193adf9891491f42 ]
Previously the cp2112 driver called INIT_DELAYED_WORK within
cp2112_gpio_irq_startup, resulting in duplicate initilizations of the
workqueue on subsequent IRQ startups following an initial request. This
resulted in a warning in set_work_data in workqueue.c, as well as a rare
NULL dereference within process_one_work in workqueue.c.
Initialize the workqueue within _probe instead.
Fixes: 13de9cca514e ("HID: cp2112: add IRQ chip handling")
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 1437e4547edf41689d7135faaca4222ef0081bc1 ]
Register the Synaptics device as a special multitouch device with certain
quirks that may improve usability of the touchpad device.
Reported-by: Rain <rain@sunshowers.io>
Closes: https://lore.kernel.org/linux-input/2bbb8e1d-1793-4df1-810f-cb0137341ff4@app.fastmail.com/
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit ffe3b7837a2bb421df84d0177481db9f52c93a71 ]
There is a slab-out-of-bounds Write bug in hid-holtek-kbd driver.
The problem is the driver assumes the device must have an input
but some malicious devices violate this assumption.
Fix this by checking hid_device's input is non-empty before its usage.
Signed-off-by: Ma Ke <make_ruc2021@163.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
commit dac501397b9d81e4782232c39f94f4307b137452 upstream.
hidpp_connect_event() has *four* time-of-check vs time-of-use (TOCTOU)
races when it races with itself.
hidpp_connect_event() primarily runs from a workqueue but it also runs
on probe() and if a "device-connected" packet is received by the hw
when the thread running hidpp_connect_event() from probe() is waiting on
the hw, then a second thread running hidpp_connect_event() will be
started from the workqueue.
This opens the following races (note the below code is simplified):
1. Retrieving + printing the protocol (harmless race):
if (!hidpp->protocol_major) {
hidpp_root_get_protocol_version()
hidpp->protocol_major = response.rap.params[0];
}
We can actually see this race hit in the dmesg in the abrt output
attached to rhbz#2227968:
[ 3064.624215] logitech-hidpp-device 0003:046D:4071.0049: HID++ 4.5 device connected.
[ 3064.658184] logitech-hidpp-device 0003:046D:4071.0049: HID++ 4.5 device connected.
Testing with extra logging added has shown that after this the 2 threads
take turn grabbing the hw access mutex (send_mutex) so they ping-pong
through all the other TOCTOU cases managing to hit all of them:
2. Updating the name to the HIDPP name (harmless race):
if (hidpp->name == hdev->name) {
...
hidpp->name = new_name;
}
3. Initializing the power_supply class for the battery (problematic!):
hidpp_initialize_battery()
{
if (hidpp->battery.ps)
return 0;
probe_battery(); /* Blocks, threads take turns executing this */
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
}
4. Creating delayed input_device (potentially problematic):
if (hidpp->delayed_input)
return;
hidpp->delayed_input = hidpp_allocate_input(hdev);
The really big problem here is 3. Hitting the race leads to the following
sequence:
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
...
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
So now we have registered 2 power supplies for the same battery,
which looks a bit weird from userspace's pov but this is not even
the really big problem.
Notice how:
1. This is all devm-maganaged
2. The hidpp->battery.desc struct is shared between the 2 power supplies
3. hidpp->battery.desc.properties points to the result from the second
devm_kmemdup()
This causes a use after free scenario on USB disconnect of the receiver:
1. The last registered power supply class device gets unregistered
2. The memory from the last devm_kmemdup() call gets freed,
hidpp->battery.desc.properties now points to freed memory
3. The first registered power supply class device gets unregistered,
this involves sending a remove uevent to userspace which invokes
power_supply_uevent() to fill the uevent data
4. power_supply_uevent() uses hidpp->battery.desc.properties which
now points to freed memory leading to backtraces like this one:
Sep 22 20:01:35 eric kernel: BUG: unable to handle page fault for address: ffffb2140e017f08
...
Sep 22 20:01:35 eric kernel: Workqueue: usb_hub_wq hub_event
Sep 22 20:01:35 eric kernel: RIP: 0010:power_supply_uevent+0xee/0x1d0
...
Sep 22 20:01:35 eric kernel: ? asm_exc_page_fault+0x26/0x30
Sep 22 20:01:35 eric kernel: ? power_supply_uevent+0xee/0x1d0
Sep 22 20:01:35 eric kernel: ? power_supply_uevent+0x10d/0x1d0
Sep 22 20:01:35 eric kernel: dev_uevent+0x10f/0x2d0
Sep 22 20:01:35 eric kernel: kobject_uevent_env+0x291/0x680
Sep 22 20:01:35 eric kernel: power_supply_unregister+0x8e/0xa0
Sep 22 20:01:35 eric kernel: release_nodes+0x3d/0xb0
Sep 22 20:01:35 eric kernel: devres_release_group+0xfc/0x130
Sep 22 20:01:35 eric kernel: hid_device_remove+0x56/0xa0
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? __queue_work+0x1df/0x440
Sep 22 20:01:35 eric kernel: hid_destroy_device+0x4b/0x60
Sep 22 20:01:35 eric kernel: logi_dj_remove+0x9a/0x100 [hid_logitech_dj 5c91534a0ead2b65e04dd799a0437e3b99b21bc4]
Sep 22 20:01:35 eric kernel: hid_device_remove+0x44/0xa0
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? __queue_work+0x1df/0x440
Sep 22 20:01:35 eric kernel: hid_destroy_device+0x4b/0x60
Sep 22 20:01:35 eric kernel: usbhid_disconnect+0x47/0x60 [usbhid 727dcc1c0b94e6b4418727a468398ac3bca492f3]
Sep 22 20:01:35 eric kernel: usb_unbind_interface+0x90/0x270
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? kobject_put+0xa0/0x1d0
Sep 22 20:01:35 eric kernel: usb_disable_device+0xcd/0x1e0
Sep 22 20:01:35 eric kernel: usb_disconnect+0xde/0x2c0
Sep 22 20:01:35 eric kernel: usb_disconnect+0xc3/0x2c0
Sep 22 20:01:35 eric kernel: hub_event+0xe80/0x1c10
There have been quite a few bug reports (see Link tags) about this crash.
Fix all the TOCTOU issues, including the really bad power-supply related
system crash on USB disconnect, by making probe() use the workqueue for
running hidpp_connect_event() too, so that it can never run more then once.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227221
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227968
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227968
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2242189
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412#c58
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231005182638.3776-1-hdegoede@redhat.com
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
