greybus-dev April 2026

greybus-dev@lists.linaro.org

17 participants
14 discussions

[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>

4 months

[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 months, 2 weeks

[PATCH v3] staging: greybus: bootrom: replace dev_info with dev_dbg for firmware name

by Bentley Blacketer

The original dev_info call was intentionally temporary, with a FIXME comment noting it should be downgraded to dev_dbg once modules with valid VID/PID values were common. Project Ara was cancelled in 2016, so that time has long passed. Remove the FIXME comment and downgrade to dev_dbg as originally intended. Tested via code inspection only, as Project Ara hardware is no longer available. Signed-off-by: Bentley Blacketer <sonionwhat(a)gmail.com> Signed-off-by: Bentley Blacketer <sonionwhat(a)gmail.com> --- drivers/staging/greybus/bootrom.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c..058044ba7 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -163,18 +163,13 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) * * XXX Name it properly.. */ - snprintf(firmware_name, sizeof(firmware_name), +snprintf(firmware_name, sizeof(firmware_name), FW_NAME_PREFIX "%08x_%08x_%08x_%08x_s2l.tftf", intf->ddbl1_manufacturer_id, intf->ddbl1_product_id, intf->vendor_id, intf->product_id); - // FIXME: - // Turn to dev_dbg later after everyone has valid bootloaders with good - // ids, but leave this as dev_info for now to make it easier to track - // down "empty" vid/pid modules. - dev_info(&connection->bundle->dev, "Firmware file '%s' requested\n", + dev_dbg(&connection->bundle->dev, "Firmware file '%s' requested\n", firmware_name); - rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); if (rc) { -- 2.54.0

5 months

[PATCH] staging: greybus: add missing newline to sysfs_emit

by Ajith P V

The sysfs attribute is missing a terminating newline. According to the kernel documentation for sysfs, attributes should be terminated by a newline to follow standard userspace conventions. Fix the formatting by adding a '\n' to the sysfs_emit string. This resolves the checkpatch warning: "return sysfs_emit(...) formats should include a terminating newline" Signed-off-by: Ajith P V <ajithpv.linux(a)gmail.com> --- drivers/staging/greybus/audio_manager_module.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c index dc90cc2d2308..5737f2a32f5a 100644 --- a/drivers/staging/greybus/audio_manager_module.c +++ b/drivers/staging/greybus/audio_manager_module.c @@ -75,7 +75,7 @@ static void gb_audio_module_release(struct kobject *kobj) static ssize_t gb_audio_module_name_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%s", module->desc.name); + return sysfs_emit(buf, "%s\n", module->desc.name); } static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute = @@ -84,7 +84,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute = static ssize_t gb_audio_module_vid_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.vid); + return sysfs_emit(buf, "%d\n", module->desc.vid); } static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute = @@ -93,7 +93,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute = static ssize_t gb_audio_module_pid_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.pid); + return sysfs_emit(buf, "%d\n", module->desc.pid); } static struct gb_audio_manager_module_attribute gb_audio_module_pid_attribute = @@ -103,7 +103,7 @@ static ssize_t gb_audio_module_intf_id_show(struct gb_audio_manager_module *modu struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.intf_id); + return sysfs_emit(buf, "%d\n", module->desc.intf_id); } static struct gb_audio_manager_module_attribute @@ -114,7 +114,7 @@ static ssize_t gb_audio_module_ip_devices_show(struct gb_audio_manager_module *m struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "0x%X", module->desc.ip_devices); + return sysfs_emit(buf, "0x%X\n", module->desc.ip_devices); } static struct gb_audio_manager_module_attribute @@ -125,7 +125,7 @@ static ssize_t gb_audio_module_op_devices_show(struct gb_audio_manager_module *m struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "0x%X", module->desc.op_devices); + return sysfs_emit(buf, "0x%X\n", module->desc.op_devices); } static struct gb_audio_manager_module_attribute -- 2.43.0

5 months

[PATCH v4] staging: greybus: bootrom: replace dev_info with dev_dbg for firmware name

by Bentley Blacketer

The original dev_info call was intentionally temporary, with a FIXME comment noting it should be downgraded to dev_dbg once modules with valid VID/PID values were common. Project Ara was cancelled in 2016, so that time has long passed. Remove the FIXME comment and downgrade to dev_dbg as originally intended. Tested via code inspection only, as Project Ara hardware is no longer available. Signed-off-by: Bentley Blacketer <sonionwhat(a)gmail.com> --- Changes in v4: - Fixed duplicate Signed-off-by - Fixed missing tab on snprintf line Changes in v3: - Fixed author name mismatch Changes in v2: - Fixed whitespace issues - Moved testing note below --- line drivers/staging/greybus/bootrom.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c..9e0f24d6d 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -168,13 +168,8 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) intf->ddbl1_manufacturer_id, intf->ddbl1_product_id, intf->vendor_id, intf->product_id); - // FIXME: - // Turn to dev_dbg later after everyone has valid bootloaders with good - // ids, but leave this as dev_info for now to make it easier to track - // down "empty" vid/pid modules. - dev_info(&connection->bundle->dev, "Firmware file '%s' requested\n", + dev_dbg(&connection->bundle->dev, "Firmware file '%s' requested\n", firmware_name); - rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); if (rc) { -- 2.54.0

5 months

[PATCH v2] staging: greybus: bootrom: replace dev_info with dev_dbg for firmware name

by kernelcoredev

The original dev_info call was intentionally temporary, with a FIXME comment noting it should be downgraded to dev_dbg once modules with valid VID/PID values were common. Project Ara was cancelled in 2016, so that time has long passed. Remove the FIXME comment and downgrade to dev_dbg as originally intended. Tested via code inspection only, as Project Ara hardware is no longer available. Signed-off-by: Bentley Blacketer <sonionwhat(a)gmail.com> --- drivers/staging/greybus/bootrom.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c..058044ba7 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -163,18 +163,13 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) * * XXX Name it properly.. */ - snprintf(firmware_name, sizeof(firmware_name), +snprintf(firmware_name, sizeof(firmware_name), FW_NAME_PREFIX "%08x_%08x_%08x_%08x_s2l.tftf", intf->ddbl1_manufacturer_id, intf->ddbl1_product_id, intf->vendor_id, intf->product_id); - // FIXME: - // Turn to dev_dbg later after everyone has valid bootloaders with good - // ids, but leave this as dev_info for now to make it easier to track - // down "empty" vid/pid modules. - dev_info(&connection->bundle->dev, "Firmware file '%s' requested\n", + dev_dbg(&connection->bundle->dev, "Firmware file '%s' requested\n", firmware_name); - rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); if (rc) { -- 2.54.0

5 months

[PATCH] staging: greybus: bootrom: replace dev_info with dev_dbg for firmware name

by kernelcoredev

The original dev_info call was intentionally temporary, with a FIXME comment noting it should be downgraded to dev_dbg once modules with valid VID/PID values were common. Project Ara was cancelled in 2016, so that time has long passed. Remove the FIXME comment and downgrade to dev_dbg as originally intended. Tested via code inspection only, as Project Ara hardware is no longer available. Signed-off-by: Bentley Blacketer <sonionwhat(a)gmail.com> --- drivers/staging/greybus/bootrom.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c..757460c8b 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -168,12 +168,9 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) intf->ddbl1_manufacturer_id, intf->ddbl1_product_id, intf->vendor_id, intf->product_id); - // FIXME: - // Turn to dev_dbg later after everyone has valid bootloaders with good - // ids, but leave this as dev_info for now to make it easier to track - // down "empty" vid/pid modules. - dev_info(&connection->bundle->dev, "Firmware file '%s' requested\n", - firmware_name); + + dev_dbg(&connection->bundle->dev, "Firmware file '%s' requested\n", + firmware_name); rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); -- 2.54.0

5 months

[PATCH] staging: greybus: fix typo in sysfs-bus-greybus

by Maha Maryam Javaid

Fix spelling mistake: attibute -> attribute Signed-off-by: Maha Maryam Javaid <mahamaryamjavaid(a)gmail.com> --- drivers/staging/greybus/Documentation/sysfs-bus-greybus | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/Documentation/sysfs-bus-greybus b/drivers/staging/greybus/Documentation/sysfs-bus-greybus index 2e998966cbe1..feeffe54a59b 100644 --- a/drivers/staging/greybus/Documentation/sysfs-bus-greybus +++ b/drivers/staging/greybus/Documentation/sysfs-bus-greybus @@ -26,7 +26,7 @@ Date: March 2016 KernelVersion: 4.XX Contact: Greg Kroah-Hartman <greg(a)kroah.com> Description: - Writing a non-zero argument to this attibute disables the + Writing a non-zero argument to this attribute disables the module's interfaces before physically ejecting it. What: /sys/bus/greybus/devices/N-M/module_id -- 2.34.1

5 months, 1 week

[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>

5 months, 1 week

[PATCH AUTOSEL 7.0-6.12] greybus: beagleplay: bound bootloader RX buffer copy

by Sasha Levin

From: Pengpeng Hou <pengpeng(a)iscas.ac.cn> [ Upstream commit cbc96a916b1a3be7039b0166c0fc56ec1632ba01 ] When `flashing_mode` is set, `gb_tty_receive()` routes incoming bytes to `cc1352_bootloader_rx()`. That helper appends the new bytes to the shared `rx_buffer` with `memcpy()` but does not check that the chunk fits in the remaining space first. The normal HDLC receive path already enforces `MAX_RX_HDLC`, so do the same here before appending bootloader data. If a packet would overflow the receive buffer, drop it and reset the bootloader receive state instead of copying past the end of `rx_buffer`. Signed-off-by: Pengpeng Hou <pengpeng(a)iscas.ac.cn> Link: https://patch.msgid.link/20260322031923.58013-1-pengpeng@iscas.ac.cn Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- LLM Generated explanations, may be completely bogus: I have enough information to write the full analysis. # Analysis: greybus: beagleplay: bound bootloader RX buffer copy ## PHASE 1: COMMIT MESSAGE FORENSICS **Step 1.1 - Subject parsing** Record: subsystem=`greybus/beagleplay`, action=`bound` (i.e. add bounds check), summary=add a bounds check to bootloader RX path before `memcpy` into a fixed-size buffer. **Step 1.2 - Tags** Record: - `Signed-off-by: Pengpeng Hou <pengpeng(a)iscas.ac.cn>` (author) - `Link: https://patch.msgid.link/20260322031923.58013-1-pengpeng@iscas.ac.cn` - `Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>` (greybus maintainer) - No `Fixes:`, no `Cc: stable`, no `Reported-by:`, no `Reviewed-by:`. Absence is expected. **Step 1.3 - Body analysis** Record: Bug = `cc1352_bootloader_rx()` does `memcpy(rx_buffer + rx_buffer_len, data, count)` without verifying `count` fits the remaining space; the HDLC sibling path already enforces `MAX_RX_HDLC`. Failure mode = write past end of `rx_buffer` (heap buffer overflow). **Step 1.4 - Hidden bug fix detection** Record: Subject says "bound" rather than "fix", but body explicitly describes a buffer-overflow gap and the fix mirrors a guard that already exists on the parallel HDLC path. This is unambiguously a bug fix. ## PHASE 2: DIFF ANALYSIS **Step 2.1 - Inventory** Record: 1 file (`drivers/greybus/gb-beagleplay.c`), +6/-0 lines, 1 hunk, single function `cc1352_bootloader_rx()`. Surgical single-file fix. **Step 2.2 - Code flow change** Record: Before — copy unconditionally into `bg->rx_buffer + bg->rx_buffer_len`. After — if `count > sizeof(rx_buffer) - rx_buffer_len`, log a rate-limited error, reset `rx_buffer_len`, return `count` (consume the chunk). Otherwise the original path runs. **Step 2.3 - Bug mechanism** Record: Category = memory-safety / out-of-bounds write. Mechanism: `rx_buffer` is `u8 rx_buffer[MAX_RX_HDLC]` (1 + 256 + 2 = 259 bytes) embedded in `struct gb_beagleplay`. Without the check, an inbound serdev chunk plus stale `rx_buffer_len` could write past 259 bytes into heap memory adjacent to subsequent fields (`fwl`, `flashing_mode`, completions, etc.). **Step 2.4 - Fix quality** Record: 5 lines, mirrors the existing `MAX_RX_HDLC` guard in `hdlc_rx()`. Resetting `rx_buffer_len` on overflow drops staged data, which is acceptable here because the fix path is being exercised under fault conditions; bootloader sync/transfers will retry. No new regression vectors. Returning `count` correctly tells the serdev core that the bytes were consumed. ## PHASE 3: GIT HISTORY INVESTIGATION **Step 3.1 - Blame** Record: `git log -S "cc1352_bootloader_rx" -- drivers/greybus/gb- beagleplay.c` shows the function (and bug) was introduced by `0cf7befa3ea2e` ("greybus: gb-beagleplay: Add firmware upload API"). **Step 3.2 - Fixes target** Record: No explicit `Fixes:` tag here, but the introducing commit `0cf7befa3ea2e` is in `v6.12-rc1` (`git describe --contains` = `v6.12-rc1~39^2`), so the bug exists in v6.12 and later. **Step 3.3 - File history & related changes** Record: A near-identical follow-up patch `1214bf28965ce` ("greybus: gb- beagleplay: bound bootloader receive buffering") by the same author, dated 2026-04-02, was applied later to the same function with explicit `Fixes: 0cf7befa3ea2` and `Cc: stable(a)kernel.org`. It adds a second, redundant copy of the same bound check; in current `origin/master` both checks exist back-to-back. The second commit confirms the maintainer/author both view this as a real, stable-worthy bug. **Step 3.4 - Author** Record: Pengpeng Hou — submitter, not subsystem maintainer. Patch was applied directly by Greg KH (greybus maintainer). The same author followed up with a stable-tagged version when the first lacked tags. **Step 3.5 - Dependencies** Record: Self-contained. Does not depend on any other unmerged work. The function and `rx_buffer` field have been unchanged since `0cf7befa3ea2e` was merged in v6.12. ## PHASE 4: MAILING LIST RESEARCH **Step 4.1 - Lore discussion** Record: `b4 dig -c cbc96a916b1a3` matched by patch-id, returned `https:/ /lore.kernel.org/all/20260322031923.58013-1-pengpeng(a)iscas.ac.cn/`. `b4 dig -a` shows v1 only — no other revisions. **Step 4.2 - Reviewers** Record: Cc list (per the mbox): Ayush Singh (driver author), Johan Hovold, Alex Elder, Greg KH, plus greybus-dev. No replies in the thread; Greg merged it as v1. **Step 4.3 - Bug report** Record: No `Reported-by:` or external bug-tracker link. Bug found by code inspection. No syzbot or sanitizer reference. **Step 4.4 - Related patches** Record: The same author submitted a duplicate fix `20260402054016.38587-1-pengpeng(a)iscas.ac.cn` ("greybus: gb-beagleplay: bound bootloader receive buffering") containing `Fixes: 0cf7befa3ea2` and `Cc: stable(a)vger.kernel.org`. That confirms the author's intent that the fix go to stable. **Step 4.5 - Stable list** Record: Not applicable — checked recent file history, the explicit stable nomination is in the duplicate follow-up rather than this commit. ## PHASE 5: CODE SEMANTIC ANALYSIS **Step 5.1 - Modified function** Record: `cc1352_bootloader_rx()`. **Step 5.2 - Callers** Record: Single caller — `gb_tty_receive()`, which is the `.receive_buf` member of `gb_beagleplay_ops` registered with the serdev framework (`drivers/greybus/gb-beagleplay.c:556-578`). The serdev core invokes it whenever the underlying UART (cc1352p7) hands up received bytes. The branch into `cc1352_bootloader_rx` triggers when `bg->flashing_mode` is true. **Step 5.3 - Callees** Record: `memcpy`, `memmove`, and `cc1352_bootloader_pkt_rx()`. The buggy line is the `memcpy` — pre-fix, the destination pointer can advance past the 259-byte array. **Step 5.4 - Reachability** Record: `flashing_mode` is set in `cc1352_prepare()` (`drivers/greybus/gb-beagleplay.c:882`), invoked through the kernel firmware-upload framework (sysfs `/sys/class/firmware/...`). A privileged user-space firmware update on a BeaglePlay board makes the buggy path reachable. The attacker/triggerer is therefore root- equivalent, but the consequence (heap corruption from data the cc1352 sends back, batched by the UART driver into chunks > 259 bytes) is severe. **Step 5.5 - Similar patterns** Record: The HDLC receive path (`hdlc_rx()` at line 399) already guards with `bg->rx_buffer_len < MAX_RX_HDLC`. The patch makes the bootloader path consistent with this established sibling. ## PHASE 6: STABLE TREE ANALYSIS **Step 6.1 - Buggy code in stable** Record: Verified via `git merge-base --is-ancestor 0cf7befa3ea2e <stable-branch>`: - 5.10.y / 5.15.y / 6.1.y / 6.6.y → bug NOT present (driver predates `cc1352_bootloader_rx`). - 6.12.y / 6.17.y / 6.18.y / 6.19.y → bug PRESENT. **Step 6.2 - Backport difficulty** Record: The hunk lands in `cc1352_bootloader_rx()` whose body has been unchanged since v6.12. Patch should apply cleanly to all affected stable trees. (The duplicate follow-up `1214bf28965ce` was generated against the same parent, confirming this.) **Step 6.3 - Related fixes already in stable** Record: Verified via `git show <branch>:drivers/greybus/gb-beagleplay.c | rg "count > sizeof|overflow|oversized"` — neither the current commit nor the duplicate is present in any stable branch yet. No prior fix exists for this bug in stable. ## PHASE 7: SUBSYSTEM CONTEXT **Step 7.1 - Subsystem & criticality** Record: `drivers/greybus` — niche subsystem, but this driver supports a real shipped product (BeaglePlay). PERIPHERAL criticality (only affects users of that specific board). **Step 7.2 - Subsystem activity** Record: Low churn — `git log` on the file shows ~10 changes since 2024. The function being fixed has been unchanged since its introduction in v6.12. ## PHASE 8: IMPACT AND RISK ASSESSMENT **Step 8.1 - Affected population** Record: Users of the BeaglePlay board running v6.12+ kernels who exercise the cc1352p7 firmware-update path. **Step 8.2 - Trigger conditions** Record: Requires `flashing_mode` true (firmware upload in progress; needs root) AND `count` (size of a single serdev receive chunk plus already-staged bytes) exceeding 259 bytes. UART subsystems can batch incoming bytes, so this is realistic during a firmware flash, not just under attacker control. **Step 8.3 - Failure mode severity** Record: Heap out-of-bounds write inside `struct gb_beagleplay`. Likely outcomes: kernel oops/panic, slab corruption, KASAN splat, potential local privilege/info leak. Severity = HIGH. **Step 8.4 - Risk-benefit** Record: Benefit = closes a kernel heap OOB write reachable during a normal product workflow. Risk = very low — 5 lines, mirror of existing guard, only consequence is dropping a malformed/oversized chunk during firmware flash (already an error condition). Strong net positive. ## PHASE 9: SYNTHESIS **Step 9.1 - Evidence** For: - Real out-of-bounds write in `cc1352_bootloader_rx()` confirmed by reading current code. - Buggy code present in v6.12.y, v6.17.y, v6.18.y, v6.19.y. - Tiny, surgical fix (5 lines), mirrors an existing guard. - Same author submitted a follow-up explicitly tagged `Fixes:` + `Cc: stable(a)vger.kernel.org` for the same bug — strong confirmation of stable-worthiness. - Applies cleanly to affected stable trees. Against: - Niche driver (limited user population). - No `Reported-by:` / `Cc: stable` on this specific commit (expected per instructions). - Trigger requires root (firmware upload). - Cosmetic concern: if both this and `1214bf28965ce` reach stable, you get a duplicate bound check (still functionally correct). **Step 9.2 - Stable rules checklist** 1. Obviously correct & tested? Yes — minimal logic, mirrors existing pattern; merged to mainline. 2. Real bug? Yes — reachable kernel-heap OOB write. 3. Important? Yes — heap corruption. 4. Small & contained? Yes — 6 lines, one file, one function. 5. No new features/APIs? Correct. 6. Applies to stable? Yes — file unchanged since the bug was introduced. **Step 9.3 - Exception category** Memory safety / OOB write fix. **Step 9.4 - Decision** Backport. ## Verification - [Phase 1] Read full commit message; tags inventoried — no `Fixes:`/`Cc: stable` (expected). - [Phase 2] Read current `cc1352_bootloader_rx()` (lines 532–554) and confirmed `rx_buffer` is `u8[MAX_RX_HDLC]=259`. - [Phase 2] Confirmed sibling `hdlc_rx()` already enforces `MAX_RX_HDLC` (line 399). - [Phase 3] `git log -S "cc1352_bootloader_rx" -- drivers/greybus/gb- beagleplay.c` → introduced by `0cf7befa3ea2e`. - [Phase 3] `git describe --contains 0cf7befa3ea2e` → `v6.12-rc1~39^2`. - [Phase 3] `git log origin/master --oneline -- drivers/greybus/gb- beagleplay.c` revealed duplicate follow-up `1214bf28965ce` with `Fixes:` + `Cc: stable`. - [Phase 3] `git show origin/master:drivers/greybus/gb-beagleplay.c` shows two consecutive bound-check blocks at lines 620 and 626 in master (cosmetic duplication after both commits). - [Phase 4] `b4 dig -c cbc96a916b1a3` returned the lore thread (v1 only, no replies). - [Phase 4] `b4 dig -c 1214bf28965ce -m` confirmed the follow-up patch carries `Fixes: 0cf7befa3ea2` and `Cc: stable(a)vger.kernel.org`. - [Phase 5] Read `gb_tty_receive` and `gb_beagleplay_ops` to confirm the call path: serdev `.receive_buf` → `gb_tty_receive` → `cc1352_bootloader_rx` when `flashing_mode` is set. - [Phase 5] Read `cc1352_prepare()` (lines 860–929) to confirm `flashing_mode` is set during user-initiated firmware upload via the fw_upload framework. - [Phase 6] `git merge-base --is-ancestor 0cf7befa3ea2e` against each stable-push branch — bug present in 6.12.y, 6.17.y, 6.18.y, 6.19.y; absent in 6.6.y and earlier. - [Phase 6] Searched stable branches for the bound-check string — not present anywhere yet. - [Phase 8] `rx_buffer` size = `1 + 256 + 2 = 259` (`MAX_RX_HDLC`); confirmed by reading the macro definitions at lines 23–25. - UNVERIFIED: I did not measure how often a real BeaglePlay UART chunk exceeds 259 bytes during firmware flash; treating this as a defense- in-depth fix even if the trigger is rare. Doesn't change the YES decision because the OOB write itself is real. This commit is a small, contained memory-safety fix for a real kernel- heap OOB write that exists in 6.12.y/6.17.y/6.18.y/6.19.y. The same author already submitted a duplicate version explicitly tagged `Cc: stable`, which directly corroborates that the bug is stable-material. It applies cleanly and meets every stable-kernel rule. **YES** drivers/greybus/gb-beagleplay.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 87186f891a6ac..bca3132adacde 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -535,6 +535,12 @@ 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_err_ratelimited(&bg->sd->dev, "Bootloader RX buffer overflow"); + bg->rx_buffer_len = 0; + return count; + } + memcpy(bg->rx_buffer + bg->rx_buffer_len, data, count); bg->rx_buffer_len += count; -- 2.53.0

5 months, 1 week

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev April 2026