Usbio #12
Closed
Usbio #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
USBIO driver stack allows the Host to extend the number of IOs available. It adds IOs exposed by a USB device that complies with the USBIO Bridge Protocol. The USBIO stack consists of three drivers: - USBIO Bridge driver that talks to the USB device exposing its IOs - USBIO GPIO and I2C drivers that register as gpiochip and i2c adapter respectively. Here we add the USBIO Bridge driver in charge of talking to the USB device firmware. It execute control commands through control transfer pipe. Signed-off-by: Israel Cepeda <israel.a.cepeda.lopez@intel.com> Link: https://lore.kernel.org/r/20250514231817.63764-2-siddartha.reddy.kare@intel.com Signed-off-by: Hans de Goede <hansg@kernel.org>
Adds the GPIO controller driver for USBIO using auxiliary bus. This driver registers as GPIO CHIP to access the GPIOs available to the Host through the USB device. USBIO Bridge driver send the GPIO request commands through control transfer pipe. Signed-off-by: Israel Cepeda <israel.a.cepeda.lopez@intel.com> Link: https://lore.kernel.org/r/20250514231817.63764-3-siddartha.reddy.kare@intel.com Signed-off-by: Hans de Goede <hansg@kernel.org>
Adds the I2C controller driver for USBIO using auxiliary bus. This driver registers as i2c adapter to access the I2Cs available to the Host through the USB device. USBIO Bridge driver send the I2C request commands through bulk write pipe and receives responses through bulk read pipe. Signed-off-by: Israel Cepeda <israel.a.cepeda.lopez@intel.com> Signed-off-by: Siddartha R Kare <siddartha.reddy.kare@intel.com> Link: https://lore.kernel.org/r/20250514231817.63764-4-siddartha.reddy.kare@intel.com Signed-off-by: Hans de Goede <hansg@kernel.org>
In order for ACPI instantion of i2c-clients under the adapter to work the fwnode of the adapter needs to be set. And the acpi_dev_clear_dependencies() call should be done on the fwnode of the adapter, not of the parent device. Signed-off-by: Hans de Goede <hansg@kernel.org>
There was a problem hiding this comment.
Pull Request Overview
Adds foundational support for Intel USBIO bridge and client drivers, including core API definitions, bridge implementation, and GPIO/I2C auxiliary drivers.
- Introduce
include/linux/usbio.hfor core USBIO client/bridge APIs and protocol constants - Implement USBIO bridge in
drivers/usb/misc/usbio.hwith build/Kconfig entries - Add auxiliary client drivers for GPIO (
gpio-usbio) and I2C (i2c-usbio) with accompanying build/Kconfig updates
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| include/linux/usbio.h | Core USBIO API structs, macros, and ACPI binding |
| drivers/usb/misc/usbio.h | USBIO bridge protocol definitions and device struct |
| drivers/usb/misc/Makefile | Enable usbio.o build |
| drivers/usb/misc/Kconfig | Add USB_USBIO config entry |
| drivers/i2c/busses/i2c-usbio.c | Intel USBIO I2C adapter auxiliary driver |
| drivers/i2c/busses/Makefile | Enable i2c-usbio.o build |
| drivers/i2c/busses/Kconfig | Add I2C_USBIO config entry |
| drivers/gpio/gpio-usbio.c | Intel USBIO GPIO auxiliary driver |
| drivers/gpio/Makefile | Enable gpio-usbio.o build |
| drivers/gpio/Kconfig | Add GPIO_USBIO config entry |
Comment on lines
+47
to
+50
| if (uid) | ||
| for (int i = 0; i < strlen(uid); i++) | ||
| if (!kstrtouint(&uid[i], 10, &id)) | ||
| break; |
There was a problem hiding this comment.
Avoid calling strlen() on each loop iteration; compute uid length once before the loop to reduce redundant work.
Suggested change
| if (uid) | |
| for (int i = 0; i < strlen(uid); i++) | |
| if (!kstrtouint(&uid[i], 10, &id)) | |
| break; | |
| if (uid) { | |
| size_t uid_len = strlen(uid); | |
| for (int i = 0; i < uid_len; i++) | |
| if (!kstrtouint(&uid[i], 10, &id)) | |
| break; | |
| } |
| #define USBIO_GPIO_PINCFG_SHIFT 2 | ||
| #define USBIO_GPIO_PINCFG_MASK (0x3 << USBIO_GPIO_PINCFG_SHIFT) | ||
| #define USBIO_GPIO_SET_PINCFG(pin) \ | ||
| ((pin & USBIO_GPIO_PINCFG_MASK) << USBIO_GPIO_PINCFG_SHIFT) |
There was a problem hiding this comment.
The mask already includes the shift; this double-shifts the value. Use ((pin & 0x3) << USBIO_GPIO_PINCFG_SHIFT) to correctly place the two-bit config field.
Suggested change
| ((pin & USBIO_GPIO_PINCFG_MASK) << USBIO_GPIO_PINCFG_SHIFT) | |
| ((pin & 0x3) << USBIO_GPIO_PINCFG_SHIFT) |
| uint8_t caps; | ||
| } __packed; | ||
|
|
||
| uint32_t usbio_i2c_speeds[] = { |
There was a problem hiding this comment.
Defining a non-static global array in a header causes multiple-definition errors. Move this definition to a C file or mark it extern here and define it in the implementation file.
Owner
Author
|
This pull-req was created to test Copilot code-review, see: This is not intended for merging, closing this now. |
jwrdegoede
pushed a commit
that referenced
this pull request
Jul 6, 2025
In probe appletb_kbd_probe() a "struct appletb_kbd *kbd" is allocated via devm_kzalloc() to store touch bar keyboard related data. Later on if backlight_device_get_by_name() finds a backlight device with name "appletb_backlight" a timer (kbd->inactivity_timer) is setup with appletb_inactivity_timer() and the timer is armed to run after appletb_tb_dim_timeout (60) seconds. A use-after-free is triggered when failure occurs after the timer is armed. This ultimately means probe failure occurs and as a result the "struct appletb_kbd *kbd" which is device managed memory is freed. After 60 seconds the timer will have expired and __run_timers will attempt to access the timer (kbd->inactivity_timer) however the kdb structure has been freed causing a use-after free. [ 71.636938] ================================================================== [ 71.637915] BUG: KASAN: slab-use-after-free in __run_timers+0x7ad/0x890 [ 71.637915] Write of size 8 at addr ffff8881178c5958 by task swapper/1/0 [ 71.637915] [ 71.637915] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.16.0-rc2-00318-g739a6c93cc75-dirty #12 PREEMPT(voluntary) [ 71.637915] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 [ 71.637915] Call Trace: [ 71.637915] <IRQ> [ 71.637915] dump_stack_lvl+0x53/0x70 [ 71.637915] print_report+0xce/0x670 [ 71.637915] ? __run_timers+0x7ad/0x890 [ 71.637915] kasan_report+0xce/0x100 [ 71.637915] ? __run_timers+0x7ad/0x890 [ 71.637915] __run_timers+0x7ad/0x890 [ 71.637915] ? __pfx___run_timers+0x10/0x10 [ 71.637915] ? update_process_times+0xfc/0x190 [ 71.637915] ? __pfx_update_process_times+0x10/0x10 [ 71.637915] ? _raw_spin_lock_irq+0x80/0xe0 [ 71.637915] ? _raw_spin_lock_irq+0x80/0xe0 [ 71.637915] ? __pfx__raw_spin_lock_irq+0x10/0x10 [ 71.637915] run_timer_softirq+0x141/0x240 [ 71.637915] ? __pfx_run_timer_softirq+0x10/0x10 [ 71.637915] ? __pfx___hrtimer_run_queues+0x10/0x10 [ 71.637915] ? kvm_clock_get_cycles+0x18/0x30 [ 71.637915] ? ktime_get+0x60/0x140 [ 71.637915] handle_softirqs+0x1b8/0x5c0 [ 71.637915] ? __pfx_handle_softirqs+0x10/0x10 [ 71.637915] irq_exit_rcu+0xaf/0xe0 [ 71.637915] sysvec_apic_timer_interrupt+0x6c/0x80 [ 71.637915] </IRQ> [ 71.637915] [ 71.637915] Allocated by task 39: [ 71.637915] kasan_save_stack+0x33/0x60 [ 71.637915] kasan_save_track+0x14/0x30 [ 71.637915] __kasan_kmalloc+0x8f/0xa0 [ 71.637915] __kmalloc_node_track_caller_noprof+0x195/0x420 [ 71.637915] devm_kmalloc+0x74/0x1e0 [ 71.637915] appletb_kbd_probe+0x37/0x3c0 [ 71.637915] hid_device_probe+0x2d1/0x680 [ 71.637915] really_probe+0x1c3/0x690 [ 71.637915] __driver_probe_device+0x247/0x300 [ 71.637915] driver_probe_device+0x49/0x210 [...] [ 71.637915] [ 71.637915] Freed by task 39: [ 71.637915] kasan_save_stack+0x33/0x60 [ 71.637915] kasan_save_track+0x14/0x30 [ 71.637915] kasan_save_free_info+0x3b/0x60 [ 71.637915] __kasan_slab_free+0x37/0x50 [ 71.637915] kfree+0xcf/0x360 [ 71.637915] devres_release_group+0x1f8/0x3c0 [ 71.637915] hid_device_probe+0x315/0x680 [ 71.637915] really_probe+0x1c3/0x690 [ 71.637915] __driver_probe_device+0x247/0x300 [ 71.637915] driver_probe_device+0x49/0x210 [...] The root cause of the issue is that the timer is not disarmed on failure paths leading to it remaining active and accessing freed memory. To fix this call timer_delete_sync() to deactivate the timer. Another small issue is that timer_delete_sync is called unconditionally in appletb_kbd_remove(), fix this by checking for a valid kbd->backlight_dev before calling timer_delete_sync. Fixes: 93a0fc4 ("HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz <qasdev00@gmail.com> Reviewed-by: Aditya Garg <gargaditya08@live.com> Signed-off-by: Jiri Kosina <jkosina@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.