From: Greg Kroah-Hartman Date: Thu, 12 Oct 2023 17:37:57 +0000 (+0200) Subject: 4.14-stable patches X-Git-Tag: v6.1.58~38 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ee062038eeceef2d6d65be787efb42fa42650cec;p=thirdparty%2Fkernel%2Fstable-queue.git 4.14-stable patches added patches: hid-logitech-hidpp-fix-kernel-crash-on-receiver-usb-disconnect.patch --- diff --git a/queue-4.14/hid-logitech-hidpp-fix-kernel-crash-on-receiver-usb-disconnect.patch b/queue-4.14/hid-logitech-hidpp-fix-kernel-crash-on-receiver-usb-disconnect.patch new file mode 100644 index 00000000000..d66ff2124ec --- /dev/null +++ b/queue-4.14/hid-logitech-hidpp-fix-kernel-crash-on-receiver-usb-disconnect.patch @@ -0,0 +1,177 @@ +From dac501397b9d81e4782232c39f94f4307b137452 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Thu, 5 Oct 2023 20:26:38 +0200 +Subject: HID: logitech-hidpp: Fix kernel crash on receiver USB disconnect + +From: Hans de Goede + +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 +Link: https://lore.kernel.org/r/20231005182638.3776-1-hdegoede@redhat.com +Signed-off-by: Benjamin Tissoires +Signed-off-by: Greg Kroah-Hartman +--- + drivers/hid/hid-logitech-hidpp.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +--- a/drivers/hid/hid-logitech-hidpp.c ++++ b/drivers/hid/hid-logitech-hidpp.c +@@ -3128,7 +3128,8 @@ static int hidpp_probe(struct hid_device + /* Allow incoming packets */ + hid_device_io_start(hdev); + +- hidpp_connect_event(hidpp); ++ schedule_work(&hidpp->work); ++ flush_work(&hidpp->work); + + return ret; + diff --git a/queue-4.14/platform-x86-hp-wmi-mark-driver-struct-with-__refdat.patch b/queue-4.14/platform-x86-hp-wmi-mark-driver-struct-with-__refdat.patch index e23e6f4b332..2af10a94771 100644 --- a/queue-4.14/platform-x86-hp-wmi-mark-driver-struct-with-__refdat.patch +++ b/queue-4.14/platform-x86-hp-wmi-mark-driver-struct-with-__refdat.patch @@ -24,14 +24,12 @@ Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede Signed-off-by: Sasha Levin --- - drivers/platform/x86/hp-wmi.c | 8 +++++++- + drivers/platform/x86/hp-wmi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) -diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c -index c65b800310f3a..67a9f838dfb9d 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c -@@ -1001,7 +1001,13 @@ static const struct dev_pm_ops hp_wmi_pm_ops = { +@@ -1001,7 +1001,13 @@ static const struct dev_pm_ops hp_wmi_pm .restore = hp_wmi_resume_handler, }; @@ -46,6 +44,3 @@ index c65b800310f3a..67a9f838dfb9d 100644 .driver = { .name = "hp-wmi", .pm = &hp_wmi_pm_ops, --- -2.40.1 - diff --git a/queue-4.14/series b/queue-4.14/series index ac8ab4a8478..0d33b6f1ccf 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -1,2 +1,3 @@ rdma-cxgb4-check-skb-value-for-failure-to-allocate.patch platform-x86-hp-wmi-mark-driver-struct-with-__refdat.patch +hid-logitech-hidpp-fix-kernel-crash-on-receiver-usb-disconnect.patch