greybus-dev April 2026

greybus-dev@lists.linaro.org

11 participants
7 discussions

[PATCH v2 0/4] HID: Proper fix for OOM in hid-core

by Benjamin Tissoires

Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()") enforced the provided data to be at least the size of the declared buffer in the report descriptor to prevent a buffer overflow. We only had corner cases of malicious devices exposing the OOM because in most cases, the buffer provided by the transport layer needs to be allocated at probe time and is large enough to handle all the possible reports. However, the patch from above, which enforces the spec a little bit more introduced both regressions for devices not following the spec (not necesserally malicious), but also a stream of errors for those devices. Let's revert to the old behavior by giving more information to HID core to be able to decide whether it can or not memset the rest of the buffer to 0 and continue the processing. Note that the first commit makes an API change, but the callers are relatively limited, so it should be fine on its own. The second patch can't really make the same kind of API change because we have too many callers in various subsystems. We can switch them one by one to the safe approach when needed. The last 2 patches are small cleanups I initially put together with the 2 first patches, but they can be applied on their own and don't need to be pulled in stable like the first 2. Cheers, Benjamin Signed-off-by: Benjamin Tissoires <bentiss(a)kernel.org> --- Changes in v2: - added a small blurb explaining the difference between the safe and the non safe version of hid_safe_input_report - Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org --- Benjamin Tissoires (4): HID: pass the buffer size to hid_report_raw_event HID: core: introduce hid_safe_input_report() HID: multitouch: use __free(kfree) to clean up temporary buffers HID: wacom: use __free(kfree) to clean up temporary buffers drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++-- drivers/hid/hid-core.c | 67 ++++++++++++++++++++++++++++++-------- drivers/hid/hid-gfrm.c | 4 +-- drivers/hid/hid-logitech-hidpp.c | 2 +- drivers/hid/hid-multitouch.c | 18 ++++------ drivers/hid/hid-primax.c | 2 +- drivers/hid/hid-vivaldi-common.c | 2 +- drivers/hid/i2c-hid/i2c-hid-core.c | 7 ++-- drivers/hid/usbhid/hid-core.c | 11 ++++--- drivers/hid/wacom_sys.c | 46 +++++++++----------------- drivers/staging/greybus/hid.c | 2 +- include/linux/hid.h | 6 ++-- include/linux/hid_bpf.h | 14 +++++--- 13 files changed, 109 insertions(+), 78 deletions(-) --- base-commit: 7df6572f1cb381d6b89ceed58e3b076c233c2cd0 change-id: 20260415-wip-fix-core-7d85c8516ed0 Best regards, -- Benjamin Tissoires <bentiss(a)kernel.org>

3 days, 7 hours

[PATCH] staging: greybus: uart: document c_cflag handling in set_termios

by Debjeet Banerjee

gb_tty_set_termios() derives UART line configuration from a subset of termios->c_cflag bits, namely CSIZE, CSTOPB, PARENB, PARODD, CMSPAR, CRTSCTS, CLOCAL and CBAUD. Other c_cflag bits are not interpreted by the driver and are not represented in the Greybus UART protocol messages. The existing FIXME suggests clearing unsupported bits from termios. However, the driver already limits its behavior to the supported subset when constructing line coding, and unused bits are effectively ignored. No invalid or unsupported values are propagated to the hardware. Replace the FIXME with a comment documenting which c_cflag bits are consumed by the driver and clarifying that other bits are ignored. No functional change intended. Signed-off-by: Debjeet Banerjee <debjeetbanerjee48(a)gmail.com> --- drivers/staging/greybus/uart.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..49d685a6ad8c 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -494,8 +494,20 @@ static void gb_tty_set_termios(struct tty_struct *tty, (termios->c_cflag & CMSPAR ? 2 : 0) : 0; newline.data_bits = tty_get_char_size(termios->c_cflag); - - /* FIXME: needs to clear unsupported bits in the termios */ + /* + * The Greybus UART driver only interprets a subset of termios + * c_cflag bits when configuring line settings: + * + * - CSIZE via tty_get_char_size() for data bits + * - CSTOPB for stop-bit format + * - PARENB, PARODD, CMSPAR for parity encoding + * - CRTSCTS for hardware flow control + * - CLOCAL for modem control handling + * - CBAUD via C_BAUD() for baud rate and B0 semantics + * + * Other c_cflag bits are ignored as they are not represented in + * the Greybus UART protocol. + */ gb_tty->clocal = ((termios->c_cflag & CLOCAL) != 0); if (C_BAUD(tty) == B0) { -- 2.53.0

4 days, 12 hours

[PATCH 0/4] HID: Proper fix for OOM in hid-core

by Benjamin Tissoires

Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()") enforced the provided data to be at least the size of the declared buffer in the report descriptor to prevent a buffer overflow. We only had corner cases of malicious devices exposing the OOM because in most cases, the buffer provided by the transport layer needs to be allocated at probe time and is large enough to handle all the possible reports. However, the patch from above, which enforces the spec a little bit more introduced both regressions for devices not following the spec (not necesserally malicious), but also a stream of errors for those devices. Let's revert to the old behavior by giving more information to HID core to be able to decide whether it can or not memset the rest of the buffer to 0 and continue the processing. Note that the first commit makes an API change, but the callers are relatively limited, so it should be fine on its own. The second patch can't really make the same kind of API change because we have too many callers in various subsystems. We can switch them one by one to the safe approach when needed. The last 2 patches are small cleanups I initially put together with the 2 first patches, but they can be applied on their own and don't need to be pulled in stable like the first 2. Cheers, Benjamin Signed-off-by: Benjamin Tissoires <bentiss(a)kernel.org> --- Benjamin Tissoires (4): HID: pass the buffer size to hid_report_raw_event HID: core: introduce hid_safe_input_report() HID: multitouch: use __free(kfree) to clean up temporary buffers HID: wacom: use __free(kfree) to clean up temporary buffers drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++-- drivers/hid/hid-core.c | 63 +++++++++++++++++++++++++++++--------- drivers/hid/hid-gfrm.c | 4 +-- drivers/hid/hid-logitech-hidpp.c | 2 +- drivers/hid/hid-multitouch.c | 18 +++++------ drivers/hid/hid-primax.c | 2 +- drivers/hid/hid-vivaldi-common.c | 2 +- drivers/hid/i2c-hid/i2c-hid-core.c | 7 +++-- drivers/hid/usbhid/hid-core.c | 11 ++++--- drivers/hid/wacom_sys.c | 46 ++++++++++------------------ drivers/staging/greybus/hid.c | 2 +- include/linux/hid.h | 6 ++-- include/linux/hid_bpf.h | 14 ++++++--- 13 files changed, 105 insertions(+), 78 deletions(-) --- base-commit: 7df6572f1cb381d6b89ceed58e3b076c233c2cd0 change-id: 20260415-wip-fix-core-7d85c8516ed0 Best regards, -- Benjamin Tissoires <bentiss(a)kernel.org>

1 week

[PATCH] greybus: audio: extract topology size retrieval into a helper

by Hungyu Lin

The topology retrieval code currently mixes the size query with allocation and data fetch in a single function. Extract the size query into a helper to make the control flow clearer. No functional change. Signed-off-by: Hungyu Lin <dennylin0707(a)gmail.com> --- drivers/staging/greybus/audio_gb.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 9d8994fdb41a..d95f8b7d4ce2 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -8,21 +8,33 @@ #include <linux/greybus.h> #include "audio_codec.h" +static int gb_audio_gb_get_topology_size(struct gb_connection *connection, + u16 *size) +{ + struct gb_audio_get_topology_size_response size_resp; + int ret; + + ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, + NULL, 0, &size_resp, sizeof(size_resp)); + if (ret) + return ret; + + *size = le16_to_cpu(size_resp.size); + return 0; +} + /* TODO: Split into separate calls */ int gb_audio_gb_get_topology(struct gb_connection *connection, struct gb_audio_topology **topology) { - struct gb_audio_get_topology_size_response size_resp; struct gb_audio_topology *topo; u16 size; int ret; - ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, - NULL, 0, &size_resp, sizeof(size_resp)); + ret = gb_audio_gb_get_topology_size(connection, &size); if (ret) return ret; - size = le16_to_cpu(size_resp.size); if (size < sizeof(*topo)) return -ENODATA; -- 2.34.1

1 week, 3 days

[PATCH] staging: greybus: authenticate: fix alignment and type warning

by Kosugi Souta

Fix the following checkpatch.pl warning and check: - Warning: Prefer 'unsigned long long' over 'unsigned long long int' - Check: Alignment should match open parenthesis Signed-off-by: Kosugi Souta <k.souta0926(a)gmail.com> --- drivers/staging/greybus/Documentation/firmware/authenticate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c index 3d2c6f88a138..ba4b16b04557 100644 --- a/drivers/staging/greybus/Documentation/firmware/authenticate.c +++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c @@ -58,7 +58,7 @@ int main(int argc, char *argv[]) goto close_fd; } - printf("UID received: 0x%llx\n", *(unsigned long long int *)(uid.uid)); + printf("UID received: 0x%llx\n", *(unsigned long long *)(uid.uid)); /* Get certificate */ printf("Get IMS certificate\n"); @@ -85,7 +85,7 @@ int main(int argc, char *argv[]) } printf("Authenticated, result (%02x), sig-size (%02x)\n", - authenticate.result_code, authenticate.signature_size); + authenticate.result_code, authenticate.signature_size); close_fd: close(fd); -- 2.34.1

2 weeks, 5 days

[PATCH 1/2] greybus: gb-beagleplay: fix sleep in atomic context in hdlc_tx_frames()

by Weigang He

hdlc_append() calls usleep_range() to wait for circular buffer space, but it is called with tx_producer_lock (a spinlock) held via hdlc_tx_frames() -> hdlc_append_tx_frame()/hdlc_append_tx_u8()/etc. Sleeping while holding a spinlock is illegal and can trigger "BUG: scheduling while atomic". Fix this by moving the buffer-space wait out of hdlc_append() and into hdlc_tx_frames(), before the spinlock is acquired. The new flow: 1. Pre-calculate the worst-case encoded frame length. 2. Wait (with sleep) outside the lock until enough space is available, kicking the TX consumer work to drain the buffer. 3. Acquire the spinlock, re-verify space, and write the entire frame atomically. This ensures that sleeping only happens without any lock held, and that frames are either fully enqueued or not written at all. This bug is found by CodeQL static analysis tool (interprocedural sleep-in-atomic query) and my code review. Fixes: ec558bbfea67 ("greybus: Add BeaglePlay Linux Driver") Cc: stable(a)vger.kernel.org Cc: Ayush Singh <ayushdevel1325(a)gmail.com> Cc: Johan Hovold <johan(a)kernel.org> Cc: Alex Elder <elder(a)kernel.org> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Weigang He <geoffreyhe2(a)gmail.com> --- drivers/greybus/gb-beagleplay.c | 105 +++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 16 deletions(-) diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 87186f891a6ac..da1b9039fd2f3 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -242,30 +242,26 @@ static void hdlc_write(struct gb_beagleplay *bg) } /** - * hdlc_append() - Queue HDLC data for sending. + * hdlc_append() - Queue a single HDLC byte for sending. * @bg: beagleplay greybus driver * @value: hdlc byte to transmit * - * Assumes that producer lock as been acquired. + * Caller must hold tx_producer_lock and must have ensured sufficient + * space in the circular buffer before calling (see hdlc_tx_frames()). */ static void hdlc_append(struct gb_beagleplay *bg, u8 value) { - int tail, head = bg->tx_circ_buf.head; + int head = bg->tx_circ_buf.head; + int tail = READ_ONCE(bg->tx_circ_buf.tail); - while (true) { - tail = READ_ONCE(bg->tx_circ_buf.tail); - - if (CIRC_SPACE(head, tail, TX_CIRC_BUF_SIZE) >= 1) { - bg->tx_circ_buf.buf[head] = value; + lockdep_assert_held(&bg->tx_producer_lock); + if (WARN_ON_ONCE(CIRC_SPACE(head, tail, TX_CIRC_BUF_SIZE) < 1)) + return; - /* Finish producing HDLC byte */ - smp_store_release(&bg->tx_circ_buf.head, - (head + 1) & (TX_CIRC_BUF_SIZE - 1)); - return; - } - dev_warn(&bg->sd->dev, "Tx circ buf full"); - usleep_range(3000, 5000); - } + bg->tx_circ_buf.buf[head] = value; + /* Ensure buffer write is visible before advancing head. */ + smp_store_release(&bg->tx_circ_buf.head, + (head + 1) & (TX_CIRC_BUF_SIZE - 1)); } static void hdlc_append_escaped(struct gb_beagleplay *bg, u8 value) @@ -313,13 +309,90 @@ static void hdlc_transmit(struct work_struct *work) spin_unlock_bh(&bg->tx_consumer_lock); } +/** + * hdlc_encoded_length() - Calculate worst-case encoded length of an HDLC frame. + * @payloads: array of payload buffers + * @count: number of payloads + * + * Returns the maximum number of bytes needed in the circular buffer. + */ +static size_t hdlc_encoded_length(const struct hdlc_payload payloads[], + size_t count) +{ + size_t i, payload_len = 0; + + for (i = 0; i < count; i++) + payload_len += payloads[i].len; + + /* + * Worst case: every data byte needs escaping (doubles in size). + * data bytes = address(1) + control(1) + payload + crc(2) + * framing = opening flag(1) + closing flag(1) + */ + return 2 + (1 + 1 + payload_len + 2) * 2; +} + +#define HDLC_TX_BUF_WAIT_RETRIES 500 +#define HDLC_TX_BUF_WAIT_US_MIN 3000 +#define HDLC_TX_BUF_WAIT_US_MAX 5000 + +/** + * hdlc_tx_frames() - Encode and queue an HDLC frame for transmission. + * @bg: beagleplay greybus driver + * @address: HDLC address field + * @control: HDLC control field + * @payloads: array of payload buffers + * @count: number of payloads + * + * Sleeps outside the spinlock until enough circular-buffer space is + * available, then verifies space under the lock and writes the entire + * frame atomically. Either a complete frame is enqueued or nothing is + * written, avoiding both sleeping in atomic context and partial frames. + */ static void hdlc_tx_frames(struct gb_beagleplay *bg, u8 address, u8 control, const struct hdlc_payload payloads[], size_t count) { + size_t needed = hdlc_encoded_length(payloads, count); + int retries = HDLC_TX_BUF_WAIT_RETRIES; size_t i; + int head, tail; + + /* Wait outside the lock for sufficient buffer space. */ + while (retries--) { + /* Pairs with smp_store_release() in hdlc_append(). */ + head = smp_load_acquire(&bg->tx_circ_buf.head); + tail = READ_ONCE(bg->tx_circ_buf.tail); + + if (CIRC_SPACE(head, tail, TX_CIRC_BUF_SIZE) >= needed) + break; + + /* Kick the consumer and sleep — no lock held. */ + schedule_work(&bg->tx_work); + usleep_range(HDLC_TX_BUF_WAIT_US_MIN, HDLC_TX_BUF_WAIT_US_MAX); + } + + if (retries < 0) { + dev_warn_ratelimited(&bg->sd->dev, + "Tx circ buf full, dropping frame\n"); + return; + } spin_lock(&bg->tx_producer_lock); + /* + * Re-check under the lock. Should not fail since + * tx_producer_lock serialises all producers and the + * consumer only frees space, but guard against it. + */ + head = bg->tx_circ_buf.head; + tail = READ_ONCE(bg->tx_circ_buf.tail); + if (unlikely(CIRC_SPACE(head, tail, TX_CIRC_BUF_SIZE) < needed)) { + spin_unlock(&bg->tx_producer_lock); + dev_warn_ratelimited(&bg->sd->dev, + "Tx circ buf space lost, dropping frame\n"); + return; + } + hdlc_append_tx_frame(bg); hdlc_append_tx_u8(bg, address); hdlc_append_tx_u8(bg, control); -- 2.34.1

3 weeks

[PATCH] greybus: gb-beagleplay: bound bootloader receive buffering

by Pengpeng Hou

cc1352_bootloader_rx() appends each serdev chunk into the fixed rx_buffer before parsing bootloader packets. The helper can keep leftover bytes between callbacks and may receive multiple packets in one callback, so a single count value is not constrained by one packet length. Check that the incoming chunk fits in the remaining receive buffer space before memcpy(). If it does not, drop the staged data and consume the bytes instead of overflowing rx_buffer. Fixes: 0cf7befa3ea2 ("greybus: gb-beagleplay: Add firmware upload API") Cc: stable(a)vger.kernel.org Signed-off-by: Pengpeng Hou <pengpeng(a)iscas.ac.cn> --- drivers/greybus/gb-beagleplay.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 87186f891a6a..e70787146c4f 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -535,6 +535,13 @@ static size_t cc1352_bootloader_rx(struct gb_beagleplay *bg, const u8 *data, int ret; size_t off = 0; + if (count > sizeof(bg->rx_buffer) - bg->rx_buffer_len) { + dev_warn(&bg->sd->dev, + "dropping oversized bootloader receive chunk"); + bg->rx_buffer_len = 0; + return count; + } + memcpy(bg->rx_buffer + bg->rx_buffer_len, data, count); bg->rx_buffer_len += count; -- 2.50.1 (Apple Git-155)

3 weeks

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev April 2026