greybus-dev

greybus-dev@lists.linaro.org

18 participants
873 discussions

[PATCH] staging: greybus:add missing newline in sysfs show() output

by Shyam Sunder Reddy Padira

Sysfs show() functions should return newline-terminated strings.Add missing newline to sysfs_emit() format string. No functional changes. Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira(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

4 months, 1 week

[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

4 months, 1 week

[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

4 months, 1 week

[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

4 months, 1 week

[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

4 months, 2 weeks

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

4 months, 2 weeks

[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

4 months, 2 weeks

[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

4 months, 2 weeks

[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

5 months

[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

5 months, 1 week

← Newer
1
...
9
10
11
12
13
14
15
...
88
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev