A Greybus network needs an SVC (Supervisory Controller) to bring
interfaces up, assign device IDs and connect CPorts to the AP. On a
UniPro network the SVC is a real entity on the bus. On transports that
merely carry Greybus messages - a UART, an I2C bus, a network link -
nothing on the wire plays that role, so the SVC has to be emulated
somewhere: in a user-space bridge (gbridge), in coprocessor firmware
(cc1352p7 in gb-beagleplay), or open-coded inside the host driver itself.
This series moves the emulation into the kernel and makes it shared.
This was discussed in a prior RFC as well [2], but to keep things short,
removing the need for an external SVC greatly simplifies the greybus
network setup when nodes are directly connected over common transports
such as UART, I2C etc.
gb-uart-node gb-softsvc greybus core
(transport, serdev) <-> (SVC + host device) <-> (bundles, protocols)
NodeOps module_insert()
submit_message() module_remove()
gb-softsvc registers itself as a Greybus host device and answers the
SVC-side operations the core expects during interface bring-up and
teardown. A host driver implements the NodeOps trait to push data
towards its node, calls module_insert() to announce a new node,
submit_message() to hand incoming Greybus messages back to the core,
and module_remove() on disconnect.
The first user is gb-uart-node, a driver for Greybus nodes attached over
a plain serial port. Framing is HDLC, with a one-byte address (0x01 for
Greybus) and control byte, followed by the 16-bit CPort ID and the
Greybus message. No SVC firmware is required on the far end, so the node
can be a bare microcontroller speaking Greybus - a BeagleConnect Freedom
over its serial link, in this case. The testing is performed with
greybus-zephyr [0] implementation.
Both drivers are written in Rust, which is why the middle of the series
is abstractions rather than drivers. Only the APIs these two drivers
need are covered: protocols.rs abstracts the SVC-facing parts of
greybus_protocols.h, and types such as Greybus Interface do not
implement AlwaysRefCounted yet. The intent is to grow this as more Rust
host drivers appear rather than to abstract the whole subsystem up
front.
Patches 1 and 2 are small C-side preparations to the Greybus core:
exporting gb_connection_get()/gb_connection_put() and adding
gb_connection_hd_find_by_intf(), a lookup by remote interface and CPort
id for callers that only know the far end of a connection. Patch 3 adds
a CRC-CCITT abstraction, needed for HDLC frame checks. Patch 4 adds the
Greybus abstractions, patches 5 and 6 the two drivers, and patch 7 the
device tree binding for BeagleConnect Freedom.
Open questions
***************
- gb-uart-node imports types from gb-softsvc, so the series carries the
Rust-to-Rust cross-module calling setup from nova-core [1]:
gb_softsvc_exports.c plus the Makefile plumbing that emits crate
metadata and generates the export list. This is a workaround for the
build system not supporting Rust cross-module dependencies natively,
and it should go away once that lands.
- Connection create/destroy and interface activate/resume in gb-softsvc
currently just acknowledge the request. Callbacks into NodeOps can be
added when a transport actually needs to act on them; I did not want
to invent an interface without a user.
- Zerocopy is currently not being used in grebeybus/protocols.rs. They
cannot be derived yet since types generated by bindgen do not have
them, and it seems explicitly forbindden to manually impl the traits.
So using old traits from transmute.
- The bindings are supposed to be created for actual device, but any MCU
that supports Zephyr, can run the greybus-zephyr firmware with UART
transport. So not sure if adding a beagle,beagleconnect-freedom
compatible is the correct choice here.
- The individual patches can be spun off into independent patch series
if required. The reason for this single patch series is to provide a
complete picture of usage.
- Since gb-softsvc currently is not being used from a C driver, no C API
is provided. However, if required, it can be added.
- Writing to UART from gb-uart-node is currently a bit broken. I am not
quite sure what the safe way is to go from a non-bound device to a
bound device. Any suggestions on this front are welcome.
- I am not sure if Rust abstractions should have a seperate entry in
MAINTAINERS with me as the maintainer, or if they should just be added
to the respective subsystem entries. As mentioned by Miguel Ojeda
[3], this is upto greybus maintainers. So waiting on their inputs. I
am available to be maintainer for the Rust abstractions for greybus
if required.
[0]: https://github.com/beagleboard/greybus-zephyr
[1]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.…
[2]: https://lore.kernel.org/all/ecca8eb2-8e5a-4770-bcf6-3fb49773088b@beagleboar…
[3]: https://lore.kernel.org/all/CANiq72mA6pTmPJUk8g7y3Z=wetFMXdzHLiPEPChpY4OTg3…
Signed-off-by: Ayush Singh <ayush(a)beagleboard.org>
---
Changes in v5:
- Remove AlwaysRefCounted impl from Interface. No longer required.
- Fix invalid comments.
- Fix interface drop in spinlock.
- Impl AlwaysRefCounted for hd::Device only in normal context.
- Match the bus device struct defination in device.rs docs.
- Return GB_OP_PROTOCOL_BAD instead of ENOTSUP when encountering an
unknown message.
- Fix possible race condition in SVC module removed. Flagged by
sashiko-bot. Involved, ensuring that operation_cancel cannot drop an
in-flight message.
- Fix timer leak in a greybus operation.
- Check write_all return.
- Return E2BIG when frame lenght is exceeded in uart node driver.
- Link to v4: https://lore.kernel.org/r/20261021-gb-uart-transport-v4-0-c308ce821d45@beag…
Changes in v4:
- Remove gb_uard_node::GbUartNode::unbind. Suggested by Markus Probst,
since write is still valid after unbind.
- Add inline attribute where suggested.
- Remove patch 1. No longer required with modifications to the old patch
2.
- Increse hdlc max buffer size to 512 bytes.
- Send module_removed request before removing from map.
- Also do not hold lock across interface drop.
- Link to v3: https://lore.kernel.org/r/20261013-gb-uart-transport-v3-0-81d0a8a1acf7@beag…
Changes in v3:
- Fix incorrect email in MAINTAINERS.
- Make CONFIG_RUST_GREYBUS_ABSTRACTIONS depend on sysfs like
CONFIG_GREYBUS.
- Fix cleanup in hd::Registration::new in case of errors.
- Fix comments.
- Fix Msg AsBytes impl as suggested by sashiko-bot.
- Use SpinLock for intf_map. This should be fine since we need locking
only for getting the interface. We do not hold locks across operations.
- Properly drop message in case of buffer overflow in gb_uart_node.
- Link to v2: https://lore.kernel.org/r/20260827-gb-uart-transport-v2-0-a03bb1f5fbd1@beag…
Changes in v2:
- Fix possible null pointer dereference in
gb_connection_hd_find_by_intf. Flagged by sashiko-bot.
- Remove prompt CONFIG_RUST_CRC_CCITT_ABSTRACTIONS Kconfig symbol.
- Add CONFIG_RUST_GREYBUS_ABSTRACTIONS to ensure that greybus is
built-in for rust abstractions to work.
- Use gfp_mask in message_send callback.
- Only provide a Message reference in message_cancel callback.
- Add invariant comment for Registration.
- Call Registration->add directly in new, before constructing
Registration.
- Add Send and Sync bounds to T in Registration<T>
- Set endo_id as 0. Is not used anywhere.
- Fix module_insert intf_count. Was hardcoded to 1 by mistake. Flagged by
sashiko-bot.
- Check for empty intfs slice in Module::new.
- Remove module in serdev::Driver::unbind instead of on drop.
- Make gb_uart_node write atomic. Using temp buffer to build frame.
- Fix import style.
- Reorder beagleconnect-freedom dtbinding patch to be before gb_uart_node.
- Add vbat-supply and reg properties to beagleconnect-freedom dtbinding.
- Reference spi-peripheral-props in beagleconnect-freedom dtbinding.
- Link to v1: https://lore.kernel.org/r/20260820-gb-uart-transport-v1-0-282da14ab7b7@beag…
---
Ayush Singh (9):
greybus: operation: keep request active across message_send
greybus: svc: add gb_svc_flush()
greybus: operation: arm request timeout before sending
greybus: connection: Add greybus_data_rcvd_by_intf()
rust: crc_ccitt: add CRC-CCITT abstraction
rust: kernel: Add greybus abstractions
drivers: greybus: Add software SVC implementation
dt-bindings: beagle: Add BeagleConnect Freedom
greybus: Add Rust UART node driver
.../beagle/beagle,beagleconnect-freedom.yaml | 62 +++
MAINTAINERS | 15 +
drivers/greybus/.gitignore | 1 +
drivers/greybus/Kconfig | 35 ++
drivers/greybus/Makefile | 50 ++
drivers/greybus/connection.c | 47 ++
drivers/greybus/gb_softsvc.rs | 533 +++++++++++++++++++++
drivers/greybus/gb_softsvc_exports.c | 15 +
drivers/greybus/gb_uart_node.rs | 270 +++++++++++
drivers/greybus/operation.c | 29 +-
drivers/greybus/svc.c | 14 +
include/linux/greybus/connection.h | 2 +
include/linux/greybus/svc.h | 1 +
lib/crc/Kconfig | 7 +
rust/bindings/bindings_helper.h | 2 +
rust/kernel/alloc.rs | 6 +
rust/kernel/crc_ccitt.rs | 26 +
rust/kernel/greybus/hd.rs | 391 +++++++++++++++
rust/kernel/greybus/mod.rs | 200 ++++++++
rust/kernel/greybus/protocols.rs | 445 +++++++++++++++++
rust/kernel/lib.rs | 4 +
21 files changed, 2152 insertions(+), 3 deletions(-)
---
base-commit: becb95e45ef0dda9ffb35156d909d3da7a2789c3
change-id: 20260810-gb-uart-transport-9255d6557c4c
Best regards,
--
Ayush Singh <ayush(a)beagleboard.org>
On Tue, Nov 03, 2026 at 12:28:19PM +0300, Dan Carpenter wrote:
> On Tue, Nov 03, 2026 at 10:12:50AM +0100, Johan Hovold wrote:
> > It's the kfifo_out_peek() in gb_uart_tx_write_work() that may write 2
> > bytes beyond gb_tty->buffer (as its size is currently just
> > gb_tty->buffer_payload_max).
>
> Ah, right.
>
> The right way to fix this is to have a commit message that
> says. "The gb_tty->buffer pointer is supposed to hold a struct with
> a flex array at the end, but gb_uart_probe() only allocates enough
> space for the flex array so it's two bytes too small.
>
> Part of the reason for this bug is because it uses a void pointer
> instead of a struct pointer. Clean that up as well to prevent
> future bugs."
>
> Please send something like this in v2. (Send exactly this code
> but edit the commit message if you want or leave it as-is).
Looks good, but since kzalloc_flex() was introduced in 7.0 (and this
bugs goes back to 4.9 it may be better to do that conversion in a
follow-up patch to assist the backporters.
Johan
On Tue, Nov 03, 2026 at 11:44:55AM +0300, Dan Carpenter wrote:
> On Mon, Nov 02, 2026 at 03:33:58PM -0600, Alex Elder wrote:
> > On 11/2/26 1:53 PM, Maritu Hone wrote:
> > > gb_tty->buffer is allocated once in gb_uart_probe() and reused for
> > > every outgoing chunk as the backing store for a
> > > struct gb_uart_send_data_request:
> > >
> > > struct gb_uart_send_data_request {
> > > __le16 size;
> > > __u8 data[];
> > > } __packed;
> > >
> > > In gb_uart_tx_write_work():
> > >
> > > request = gb_tty->buffer;
> > > ...
> > > send_size = gb_tty->buffer_payload_max;
> > > if (send_size > gb_tty->credits)
> > > send_size = gb_tty->credits;
> > > send_size = kfifo_out_peek(&gb_tty->write_fifo,
> > > &request->data[0], send_size);
> Wow.. Heh... I have read both the original message and your
> message and I still have no idea which function contains the
> actual memcpy() overflow. :P
It's the kfifo_out_peek() in gb_uart_tx_write_work() that may write 2
bytes beyond gb_tty->buffer (as its size is currently just
gb_tty->buffer_payload_max).
Johan
On Mon, Nov 02, 2026 at 10:53:32PM +0300, Maritu Hone wrote:
> gb_tty->buffer is allocated once in gb_uart_probe() and reused for
> every outgoing chunk as the backing store for a
> struct gb_uart_send_data_request:
>
> struct gb_uart_send_data_request {
> __le16 size;
> __u8 data[];
> } __packed;
>
> In gb_uart_tx_write_work():
>
> request = gb_tty->buffer;
> ...
> send_size = gb_tty->buffer_payload_max;
> if (send_size > gb_tty->credits)
> send_size = gb_tty->credits;
> send_size = kfifo_out_peek(&gb_tty->write_fifo,
> &request->data[0], send_size);
>
> request->data starts 2 bytes (sizeof(request->size)) into
> gb_tty->buffer, and up to buffer_payload_max bytes are written there
> whenever enough data is queued and credits allow it. That needs
> buffer_payload_max + sizeof(struct gb_uart_send_data_request) bytes
> of backing storage, i.e. max_payload bytes.
>
> gb_tty->buffer is allocated with only buffer_payload_max bytes:
>
> gb_tty->buffer_payload_max = max_payload -
> sizeof(struct gb_uart_send_data_request);
> gb_tty->buffer = kzalloc(gb_tty->buffer_payload_max, GFP_KERNEL);
>
> so a full-size write overruns the allocation by exactly
> sizeof(struct gb_uart_send_data_request) (2) bytes, corrupting
> whatever follows it on the heap.
>
> Allocate max_payload bytes instead, which is exactly
> sizeof(struct gb_uart_send_data_request) + buffer_payload_max, so the
> buffer is large enough for the header and the full data payload it is
> used to hold.
I think the above is clear enough.
> Fixes: dd1c64ede97f ("greybus: uart: kmalloc for send_data once only")
This should also go to stable:
Cc: stable(a)vger.kernel.org
> Signed-off-by: Maritu Hone <marehone12(a)gmail.com>
> ---
> drivers/staging/greybus/uart.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> index e1b73db6a17c..7bbac2603726 100644
> --- a/drivers/staging/greybus/uart.c
> +++ b/drivers/staging/greybus/uart.c
> @@ -842,7 +842,15 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
> gb_tty->buffer_payload_max = max_payload -
> sizeof(struct gb_uart_send_data_request);
>
> - gb_tty->buffer = kzalloc(gb_tty->buffer_payload_max, GFP_KERNEL);
> + /*
> + * gb_tty->buffer is reused in gb_uart_tx_write_work() as the
> + * backing store for a struct gb_uart_send_data_request, whose
> + * variable-length data[] field holds up to buffer_payload_max
> + * bytes starting right after the struct's header. The allocation
> + * therefore needs room for the header plus buffer_payload_max
> + * bytes of data, i.e. max_payload bytes, not buffer_payload_max.
> + */
LLMs like to add comments like this, but please remove it in a v2.
> + gb_tty->buffer = kzalloc(max_payload, GFP_KERNEL);
> if (!gb_tty->buffer) {
> retval = -ENOMEM;
> goto exit_put_port;
Johan
fw_mgmt_ioctl() copies struct fw_mgmt_ioc_get_backend_version,
struct fw_mgmt_ioc_intf_load_and_validate and
struct fw_mgmt_ioc_backend_fw_update directly from userspace via
copy_from_user(). Each of these contains a fixed-size
firmware_tag[GB_FIRMWARE_U_TAG_MAX_SIZE] array that is never checked
for NUL-termination before being used.
While the array is eventually copied into a Greybus request via
strscpy_pad(), which safely bounds its read and always NUL-terminates
its destination, three error paths print the original, unterminated
userspace buffer directly with "%s":
fw_mgmt_backend_fw_version_operation():
dev_err(..., "failed to get version of %s backend firmware (%d)\n",
fw_info->firmware_tag, ret);
dev_err(..., "Firmware with tag %s is not supported by Interface\n",
fw_info->firmware_tag);
fw_mgmt_backend_fw_update_operation():
dev_err(..., "backend %s firmware update request failed (%d)\n",
tag, ret);
If a caller supplies exactly GB_FIRMWARE_U_TAG_MAX_SIZE non-NUL bytes,
"%s" has no terminator inside the array and reads past it into
adjacent kernel stack memory until it happens to find a zero byte,
leaking that memory into dmesg.
Reject the ioctl up front when firmware_tag is not NUL-terminated,
mirroring the existing check already used for the equivalent
Greybus-module-supplied field in fw_download_find_firmware().
Fixes: 013e66537273 ("greybus: fw-management: Add firmware-management protocol driver")
Signed-off-by: Maritu Hone <marehone12(a)gmail.com>
---
drivers/staging/greybus/fw-management.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 5d01da6b6eb9..2ee609dd67fb 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -419,6 +419,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
sizeof(backend_fw_info)))
return -EFAULT;
+ if (strnlen(backend_fw_info.firmware_tag,
+ GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+ GB_FIRMWARE_U_TAG_MAX_SIZE)
+ return -EINVAL;
+
ret = fw_mgmt_backend_fw_version_operation(fw_mgmt,
&backend_fw_info);
if (ret)
@@ -433,6 +438,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
if (copy_from_user(&intf_load, buf, sizeof(intf_load)))
return -EFAULT;
+ if (strnlen(intf_load.firmware_tag,
+ GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+ GB_FIRMWARE_U_TAG_MAX_SIZE)
+ return -EINVAL;
+
ret = fw_mgmt_load_and_validate_operation(fw_mgmt,
intf_load.load_method, intf_load.firmware_tag);
if (ret)
@@ -457,6 +467,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
sizeof(backend_update)))
return -EFAULT;
+ if (strnlen(backend_update.firmware_tag,
+ GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+ GB_FIRMWARE_U_TAG_MAX_SIZE)
+ return -EINVAL;
+
ret = fw_mgmt_backend_fw_update_operation(fw_mgmt,
backend_update.firmware_tag);
if (ret)
--
2.43.0
From 9114eb9d60f2c4a52f05e3a33b0d6b8ca4856cac Mon Sep 17 00:00:00 2001
From: Kurmapu Sai Devansh Prasad <btech10487.26(a)bitmesra.ac.in>
Date: Mon, 2 Nov 2026 19:11:09 +0530
Subject: [PATCH] staging: greybus: fix string termination in firmware sample
The firmware_tag string copied via strncpy() is not guaranteed to be
NUL-terminated if the source string is exactly the size of the destination
buffer. This can lead to a buffer over-read.
Since alternatives like strscpy_pad cannot be used in user space
sample code, we can make it safer by limiting the copy size to
MAX_SIZE - 1 and explicitly assigning the last byte to be a NUL
terminator. This ensures the string is always
safely terminated before being passed to the ioctl.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-n…
This is my very first patch to the Linux kernel! I've been learning the
process and would deeply appreciate any feedback from the maintainers.
Signed-off-by: Kurmapu Sai Devansh Prasad <btech10487.26(a)bitmesra.ac.in>
---
.../greybus/Documentation/firmware/firmware.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c
b/drivers/staging/greybus/Documentation/firmware/firmware.c
index 3b35ef6d4adb..a6a0988ad6fe 100644
--- a/drivers/staging/greybus/Documentation/firmware/firmware.c
+++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
@@ -64,7 +64,9 @@ static int update_intf_firmware(int fd)
intf_load.minor = 0;
strncpy((char *)&intf_load.firmware_tag, firmware_tag,
- GB_FIRMWARE_U_TAG_MAX_SIZE);
+ GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+ ((char *)&intf_load.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE - 1] =
+ '\0';
ret = ioctl(fd, FW_MGMT_IOC_INTF_LOAD_AND_VALIDATE, &intf_load);
if (ret < 0) {
@@ -102,7 +104,9 @@ static int update_backend_firmware(int fd)
printf("Getting Backend Firmware Version\n");
strncpy((char *)&backend_fw_info.firmware_tag, firmware_tag,
- GB_FIRMWARE_U_TAG_MAX_SIZE);
+ GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+ ((char *)&backend_fw_info.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE
- 1] =
+ '\0';
retry_fw_version:
ret = ioctl(fd, FW_MGMT_IOC_GET_BACKEND_FW, &backend_fw_info);
@@ -130,7 +134,9 @@ static int update_backend_firmware(int fd)
printf("Updating Backend Firmware\n");
strncpy((char *)&backend_update.firmware_tag, firmware_tag,
- GB_FIRMWARE_U_TAG_MAX_SIZE);
+ GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+ ((char *)&backend_update.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE - 1] =
+ '\0';
retry_fw_update:
backend_update.status = 0;
--
2.56.0
gb_audio_manager_dump_module() drops modules_rwsem before dereferencing
the module. A concurrent gb_audio_manager_remove() can free the module
between the lookup and the dump.
Keep the read lock held while dumping the module so removal cannot free
it during the access.
Signed-off-by: linfordWu <linfordwu(a)163.com>
---
drivers/staging/greybus/audio_manager.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/audio_manager.c b/drivers/staging/greybus/audio_manager.c
index 118ada9b909..1b91acc084f 100644
--- a/drivers/staging/greybus/audio_manager.c
+++ b/drivers/staging/greybus/audio_manager.c
@@ -117,12 +117,13 @@ int gb_audio_manager_dump_module(int id)
down_read(&modules_rwsem);
module = gb_audio_manager_get_locked(id);
- up_read(&modules_rwsem);
-
- if (!module)
+ if (!module) {
+ up_read(&modules_rwsem);
return -EINVAL;
+ }
gb_audio_manager_module_dump(module);
+ up_read(&modules_rwsem);
return 0;
}
EXPORT_SYMBOL_GPL(gb_audio_manager_dump_module);
--
2.43.0