The debugfs buffers in gb_camera (data[PAGE_SIZE], length) are written
with sprintf without any bounds checking. The four places in
gb_camera_debugfs_capabilities, gb_camera_debugfs_configure_streams and
gb_camera_debugfs_flush do:
buffer->length += sprintf(buffer->data + buffer->length, ...);
buffer->length = sprintf(buffer->data, ...);
If the formatted data ever grows (e.g., more streams, larger hex dump)
or if length is already close to PAGE_SIZE, this will overrun the
PAGE_SIZE buffer and corrupt memory. The driver is debugfs-only so
the impact is limited, but it is still a real bug and the pattern is
repeated in multiple places.
Fix it by using scnprintf with the remaining size:
scnprintf(buffer->data + buffer->length, PAGE_SIZE - buffer->length, ...)
scnprintf(buffer->data, PAGE_SIZE, ...)
This is the standard way to write to a fixed-size buffer in the
kernel. It guarantees we never write past PAGE_SIZE and will truncate
instead of overrunning, which is safe for debugfs output. The return
value still accumulates in length, which matches the existing use with
simple_read_from_buffer (it will just show truncated output rather
than corrupting).
I checked that this exact conversion has not been proposed before:
the recent greybus conversions to sysfs_emit (light.c, gbphy.c) and
fbtft/vme_tsi148 scnprintf patches do not touch camera.c at all,
and a search of lore for "gb_camera_debugfs" shows no prior patch
for these four sprintf sites.
No functional change for normal sizes, just makes the code safe if
the buffer ever fills up.
Signed-off-by: Vaibhav Agarwal <contectforbusiness(a)proton.me>
---
drivers/staging/greybus/camera.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index 62b55bb28..efc83ceff 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -890,7 +890,8 @@ static ssize_t gb_camera_debugfs_capabilities(struct gb_camera *gcam,
for (i = 0; i < size; i += 16) {
unsigned int nbytes = min_t(unsigned int, size - i, 16);
- buffer->length += sprintf(buffer->data + buffer->length,
+ buffer->length += scnprintf(buffer->data + buffer->length,
+ PAGE_SIZE - buffer->length,
"%*ph\n", nbytes, caps + i);
}
@@ -973,12 +974,13 @@ static ssize_t gb_camera_debugfs_configure_streams(struct gb_camera *gcam,
if (ret < 0)
goto done;
- buffer->length = sprintf(buffer->data, "%u;%u;", nstreams, flags);
+ buffer->length = scnprintf(buffer->data, PAGE_SIZE, "%u;%u;", nstreams, flags);
for (i = 0; i < nstreams; ++i) {
struct gb_camera_stream_config *stream = &streams[i];
- buffer->length += sprintf(buffer->data + buffer->length,
+ buffer->length += scnprintf(buffer->data + buffer->length,
+ PAGE_SIZE - buffer->length,
"%u;%u;%u;%u;%u;%u;%u;",
stream->width, stream->height,
stream->format, stream->vc,
@@ -1046,7 +1048,7 @@ static ssize_t gb_camera_debugfs_flush(struct gb_camera *gcam,
if (ret < 0)
return ret;
- buffer->length = sprintf(buffer->data, "%u", req_id);
+ buffer->length = scnprintf(buffer->data, PAGE_SIZE, "%u", req_id);
return len;
}
Adhere to Linux kernel coding style. Reported by checkpatch:
CHECK: Alignment should match open parenthesis
Signed-off-by: S Tarun Kumar Lywait <tarun.k.lywait(a)gmail.com>
---
drivers/staging/greybus/camera.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index 62b55bb28408..9853bd60ff27 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -263,9 +263,10 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
* Validate the stream configuration response verifying padding is correctly
* set and the returned number of streams is supported
*/
-static const int gb_camera_configure_streams_validate_response(struct gb_camera *gcam,
- struct gb_camera_configure_streams_response *resp,
- unsigned int nstreams)
+static const int gb_camera_configure_streams_validate_response
+ (struct gb_camera *gcam,
+ struct gb_camera_configure_streams_response *resp,
+ unsigned int nstreams)
{
unsigned int i;
--
2.47.3
The following has been changed:
- introduce a new C serdev API to fix race conditions and simplify rust
code. Should also be useful on the C side.
- fix race conditions
- simplify rust code
- add rust function to provide mutable references to driver's private
data
- provide mutable references in callbacks to avoid the need for a
SpinLock in the greybus patch series [1]
[1]
https://lore.kernel.org/rust-for-linux/20260827-gb-uart-transport-v2-7-a03b…
Signed-off-by: Markus Probst <markus.probst(a)posteo.de>
---
Markus Probst (5):
tty: serdev: Export functions to pause receive_buf callback calls
rust: serdev: Replace `active` mutex with receive pause
rust: serdev: Simplify callbacks
rust: Add `Device::drvdata_borrow_mut`
rust: serdev: Pause receive callback before calling unbind
drivers/tty/serdev/core.c | 50 +++++++++-
drivers/tty/serdev/serdev-ttyport.c | 32 ++++++
include/linux/serdev.h | 6 ++
rust/kernel/device.rs | 24 +++++
rust/kernel/serdev.rs | 189 +++++++++++++-----------------------
samples/rust/rust_driver_serdev.rs | 2 +-
6 files changed, 180 insertions(+), 123 deletions(-)
---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20260905-rust_serdev_probe_refactor-0e2b044354a7
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.
[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…
Signed-off-by: Ayush Singh <ayush(a)beagleboard.org>
---
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 (7):
greybus: connection: Export gb_connection_get() and gb_connection_put()
greybus: connection: Add gb_connection_hd_find_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 | 39 ++
MAINTAINERS | 15 +
drivers/greybus/.gitignore | 1 +
drivers/greybus/Kconfig | 34 ++
drivers/greybus/Makefile | 50 ++
drivers/greybus/connection.c | 29 +-
drivers/greybus/gb_softsvc.rs | 504 +++++++++++++++++++++
drivers/greybus/gb_softsvc_exports.c | 15 +
drivers/greybus/gb_uart_node.rs | 245 ++++++++++
include/linux/greybus/connection.h | 6 +
lib/crc/Kconfig | 7 +
rust/bindings/bindings_helper.h | 2 +
rust/kernel/alloc.rs | 5 +
rust/kernel/crc_ccitt.rs | 26 ++
rust/kernel/greybus/hd.rs | 333 ++++++++++++++
rust/kernel/greybus/mod.rs | 232 ++++++++++
rust/kernel/greybus/protocols.rs | 392 ++++++++++++++++
rust/kernel/lib.rs | 4 +
18 files changed, 1937 insertions(+), 2 deletions(-)
---
base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
change-id: 20260810-gb-uart-transport-9255d6557c4c
Best regards,
--
Ayush Singh <ayush(a)beagleboard.org>