greybus-dev June 2026

greybus-dev@lists.linaro.org

19 participants
21 discussions

staging: greybus: audio: possible out of bounds read in the topology parser

by Maoyi Xie

Hi all, I think the Greybus audio topology parser in drivers/staging/greybus/audio_topology.c can read past the topology blob when a module reports inconsistent counts. I would appreciate it if you could take a look. gb_audio_gb_get_topology() allocates the blob to a device-reported u16 size and only checks that it is at least sizeof(struct gb_audio_topology). size = le16_to_cpu(size_resp.size); if (size < sizeof(*topo)) return -ENODATA; topo = kzalloc(size, GFP_KERNEL); After that the parser trusts the interior fields. gb_generate_enum_strings() walks one C string per item with no end pointer. items = le32_to_cpu(gbenum->items); for (i = 0; i < items; i++) { strings[i] = data; while (*data != '\0') data++; data++; } items is a device le32 and there is no guarantee the blob holds that many NUL terminated strings, so the while loop runs off the end of the allocation. The same pattern is in gbaudio_tplg_process_header(), which derives the control, widget and route offsets by adding device le32 block sizes with no check against the end of the blob, and in the csize cursor advance in gbaudio_tplg_process_kcontrols() and gbaudio_tplg_create_widget(). The whole blob is device data parsed on the probe path with no privilege needed. The attacker here is a malicious or faulty Greybus audio module on the interface. I reproduced the enum string walk under KASAN on 7.1-rc7. A blob with one NUL per item stays inside the allocation. A blob with missing terminators makes KASAN report a slab out of bounds read past the buffer. I have a partial fix that passes the buffer end into the enum walk and stops on overrun. A complete fix also needs to bound the header offsets and the csize advances against the allocation size, so I wanted to ask before sending a series. Does this look like a real bug to you? If it does I am happy to send a proper patch. The parser was added in c8e6336bb3f8 ("greybus: audio: Add topology parser for GB codec"), which I think is the right Fixes tag. Thanks, Maoyi https://maoyixie.com/

2 days, 2 hours

[PATCH v2] staging: greybus: vibrator: return device_create() errors

by Alfie Varghese

gb_vibrator_probe() maps any device_create() failure to -EINVAL. This loses the real errno returned by the driver core, such as -ENOMEM, and makes probe failures harder to diagnose correctly. Return PTR_ERR(dev) instead so callers receive the actual failure reason while preserving the existing cleanup path. Signed-off-by: Alfie Varghese <alfievarghese22(a)gmail.com> --- v2: No code changes, resending as a single properly versioned patch. drivers/staging/greybus/vibrator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/vibrator.c b/drivers/staging/greybus/vibrator.c index 0ec4d317c9db..763c234fbc03 100644 --- a/drivers/staging/greybus/vibrator.c +++ b/drivers/staging/greybus/vibrator.c @@ -161,7 +161,7 @@ static int gb_vibrator_probe(struct gb_bundle *bundle, dev = device_create(&vibrator_class, &bundle->dev, MKDEV(0, 0), vib, "vibrator%d", vib->minor); if (IS_ERR(dev)) { - retval = -EINVAL; + retval = PTR_ERR(dev); goto err_ida_remove; } vib->dev = dev; -- 2.53.0

2 days, 20 hours

[PATCH] staging: greybus: vibrator: return device_create() errors

by Alfie Varghese

gb_vibrator_probe() maps any device_create() failure to -EINVAL. This loses the real errno returned by the driver core, such as -ENOMEM, and makes probe failures harder to diagnose correctly. Return PTR_ERR(dev) instead so callers receive the actual failure reason while preserving the existing cleanup path. Signed-off-by: Alfie Varghese <alfievarghese22(a)gmail.com> --- drivers/staging/greybus/vibrator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/vibrator.c b/drivers/staging/greybus/vibrator.c index 0ec4d317c..763c234fb 100644 --- a/drivers/staging/greybus/vibrator.c +++ b/drivers/staging/greybus/vibrator.c @@ -161,7 +161,7 @@ static int gb_vibrator_probe(struct gb_bundle *bundle, dev = device_create(&vibrator_class, &bundle->dev, MKDEV(0, 0), vib, "vibrator%d", vib->minor); if (IS_ERR(dev)) { - retval = -EINVAL; + retval = PTR_ERR(dev); goto err_ida_remove; } vib->dev = dev; -- 2.54.0.windows.1

2 days, 21 hours

[PATCH] staging: greybus: power_supply: remove unnecessary parentheses in return

by Tomasz Unger

return is not a function, so the parentheses around the returned expressions are not needed. Remove them to follow kernel coding style and silence checkpatch.pl RETURN_PARENTHESES warnings. Verified with checkpatch.pl - no errors or warnings. Compiled the power_supply object successfully. Signed-off-by: Tomasz Unger <tomasz.unger(a)yahoo.pl> --- drivers/staging/greybus/power_supply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/power_supply.c b/drivers/staging/greybus/power_supply.c index 33b7a63979d6..44bd8a72fa50 100644 --- a/drivers/staging/greybus/power_supply.c +++ b/drivers/staging/greybus/power_supply.c @@ -336,7 +336,7 @@ static int is_psy_prop_writeable(struct gb_power_supply *gbpsy, static int is_prop_valint(enum power_supply_property psp) { - return ((psp < POWER_SUPPLY_PROP_MODEL_NAME) ? 1 : 0); + return (psp < POWER_SUPPLY_PROP_MODEL_NAME) ? 1 : 0; } static void next_interval(struct gb_power_supply *gbpsy) @@ -423,7 +423,7 @@ static void check_changed(struct gb_power_supply *gbpsy, static int total_props(struct gb_power_supply *gbpsy) { /* this return the intval plus the strval properties */ - return (gbpsy->properties_count + gbpsy->properties_count_str); + return gbpsy->properties_count + gbpsy->properties_count_str; } static void prop_append(struct gb_power_supply *gbpsy, --- base-commit: 7cb1c5b32a2bfde961fff8d5204526b609bcb30a change-id: 20260615-greybus-power-supply-return-parens-b3dd9f4a87c7 Best regards, -- Tomasz Unger <tomasz.unger(a)yahoo.pl>

4 days, 1 hour

[PATCH] staging: greybus: uart: replace IDR with XArray

by Jack Lee

DEFINE_IDR is deprecated in favor of XArray. Convert tty_minors from IDR to XArray, replacing idr_alloc, idr_find, idr_remove and idr_destroy with their xa_alloc, xa_load, xa_erase and xa_destroy equivalents. Signed-off-by: Jack Lee <skunkolee(a)gmail.com> --- drivers/staging/greybus/uart.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..30afcd0caa14 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -22,13 +22,13 @@ #include <linux/serial.h> #include <linux/tty_driver.h> #include <linux/tty_flip.h> -#include <linux/idr.h> #include <linux/fs.h> #include <linux/kdev_t.h> #include <linux/kfifo.h> #include <linux/workqueue.h> #include <linux/completion.h> #include <linux/greybus.h> +#include <linux/xarray.h> #include "gbphy.h" @@ -67,7 +67,7 @@ struct gb_tty { }; static struct tty_driver *gb_tty_driver; -static DEFINE_IDR(tty_minors); +static DEFINE_XARRAY_ALLOC(tty_minors); static DEFINE_MUTEX(table_lock); static int gb_uart_receive_data_handler(struct gb_operation *op) @@ -342,7 +342,7 @@ static struct gb_tty *get_gb_by_minor(unsigned int minor) struct gb_tty *gb_tty; mutex_lock(&table_lock); - gb_tty = idr_find(&tty_minors, minor); + gb_tty = xa_load(&tty_minors, minor); if (gb_tty) { mutex_lock(&gb_tty->mutex); if (gb_tty->disconnected) { @@ -359,14 +359,13 @@ static struct gb_tty *get_gb_by_minor(unsigned int minor) static int alloc_minor(struct gb_tty *gb_tty) { - int minor; + int ret; - mutex_lock(&table_lock); - minor = idr_alloc(&tty_minors, gb_tty, 0, GB_NUM_MINORS, GFP_KERNEL); - mutex_unlock(&table_lock); - if (minor >= 0) - gb_tty->minor = minor; - return minor; + ret = xa_alloc(&tty_minors, &gb_tty->minor, gb_tty, + XA_LIMIT(0, GB_NUM_MINORS - 1), GFP_KERNEL); + if (ret) + return ret; + return gb_tty->minor; } static void release_minor(struct gb_tty *gb_tty) @@ -375,7 +374,7 @@ static void release_minor(struct gb_tty *gb_tty) gb_tty->minor = 0; /* Maybe should use an invalid value instead */ mutex_lock(&table_lock); - idr_remove(&tty_minors, minor); + xa_erase(&tty_minors, minor); mutex_unlock(&table_lock); } @@ -984,7 +983,7 @@ static void gb_tty_exit(void) { tty_unregister_driver(gb_tty_driver); tty_driver_kref_put(gb_tty_driver); - idr_destroy(&tty_minors); + xa_destroy(&tty_minors); } static const struct gbphy_device_id gb_uart_id_table[] = { -- 2.54.0

4 days, 7 hours

[PATCH] greybus: audio: bound the topology section sizes against the fetched size

by Bryam Vargas via B4 Relay

From: Bryam Vargas <hexlabsecurity(a)proton.me> gb_audio_gb_get_topology() fetches a topology blob of a module-supplied size, and gbaudio_tplg_parse_data() then walks it by adding the module-supplied size_dais, size_controls and size_widgets fields to form the control, widget and route section offsets. Those le32 sizes are never checked against the fetched blob, so a module reporting a small topology size but large section sizes makes the offsets point past the allocation, and parsing reads out of bounds. Reject a topology whose section sizes do not fit within the fetched size before it is parsed. Fixes: 184992e305f1 ("greybus: audio: Add Greybus Audio Device Class Protocol helper routines") Cc: stable(a)vger.kernel.org Signed-off-by: Bryam Vargas <hexlabsecurity(a)proton.me> --- I reproduced the out-of-bounds read both in-kernel under KASAN and with a userspace AddressSanitizer model of the gbaudio_tplg_process_header() offset walk. The topology blob is kzalloc(size) where size is module-supplied (a u16), and process_header() forms control_offset = &data + size_dais, widget_offset = control_offset + size_controls, etc.; the consumers then read structs at those offsets. - In-kernel (7.1.0-rc5 + KASAN): a 64-byte blob (header 24, so 40 bytes available) with size_dais = 44 makes control_offset point 4 bytes past the allocation, and reading the first control byte there trips: BUG: KASAN: slab-out-of-bounds in ...parse_topology... Read of size 1 at addr ... ... which belongs to the cache kmalloc-64 of size 64 The buggy address is located 4 bytes to the right of allocated 64-byte region The patched arm (sections rejected, -EINVAL) and an in-bounds control arm (size_dais = 8) read cleanly with no KASAN report. - ASan model (-m32 and -m64): size_dais = 4096 makes control_offset point ~4 KB past the 64-byte blob - heap-buffer-overflow READ located 4056 bytes after the region, both ABIs; patched and in-bounds clean. The source is a greybus audio module trust boundary (an attacker-supplied or compromised module reporting a malformed topology); the access is a read, and a large size_dais sends the offset far enough to fault. The reproducer (kernel module + ASan model) is available on request. --- drivers/staging/greybus/audio_gb.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 9d8994fdb41a..144591f1a512 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -37,6 +37,19 @@ int gb_audio_gb_get_topology(struct gb_connection *connection, return ret; } + /* + * The size_* fields are supplied by the module and are used by + * gbaudio_tplg_parse_data() to compute offsets into the blob; make + * sure the sections fit within the fetched topology, so walking it + * cannot read out of bounds. + */ + if ((u64)le32_to_cpu(topo->size_dais) + le32_to_cpu(topo->size_controls) + + le32_to_cpu(topo->size_widgets) + le32_to_cpu(topo->size_routes) > + size - sizeof(*topo)) { + kfree(topo); + return -EINVAL; + } + *topology = topo; return 0; --- base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66 change-id: 20260616-b4-disp-4352e8b0-45e86659956e Best regards, -- Bryam Vargas <hexlabsecurity(a)proton.me>

5 days

[PATCH] staging: greybus: Remove unused macro

by Michail Tatas

Remove unused macro as indicated by the compiler when building with make W=2 Signed-off-by: Michail Tatas <michail.tatas(a)gmail.com> --- drivers/staging/greybus/loopback.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 4d085d3cd471..7442b1c4e86c 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -167,32 +167,6 @@ static DEVICE_ATTR_RO(name##_avg) gb_loopback_ro_stats_attr(field, max, u); \ gb_loopback_ro_avg_attr(field) -#define gb_loopback_attr(field, type) \ -static ssize_t field##_show(struct device *dev, \ - struct device_attribute *attr, \ - char *buf) \ -{ \ - struct gb_loopback *gb = dev_get_drvdata(dev); \ - return sysfs_emit(buf, "%" #type "\n", gb->field); \ -} \ -static ssize_t field##_store(struct device *dev, \ - struct device_attribute *attr, \ - const char *buf, \ - size_t len) \ -{ \ - int ret; \ - struct gb_loopback *gb = dev_get_drvdata(dev); \ - mutex_lock(&gb->mutex); \ - ret = sscanf(buf, "%"#type, &gb->field); \ - if (ret != 1) \ - len = -EINVAL; \ - else \ - gb_loopback_check_attr(gb, bundle); \ - mutex_unlock(&gb->mutex); \ - return len; \ -} \ -static DEVICE_ATTR_RW(field) - #define gb_dev_loopback_ro_attr(field) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ -- 2.43.0

6 days

[PATCH v2] staging: greybus: audio: check sscanf() result directly

by abdelnasser hussein

Smatch warns: drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update() warn: sscanf doesn't return error codes sscanf() returns the number of successfully matched input items, not a negative error code. Compare the return value directly with the expected number of conversions instead of storing it in ret as an error code. Also remove the redundant else-if check for snd_soc_dapm_aif_out. The widget id is validated earlier in the function, so the remaining branch can only handle snd_soc_dapm_aif_out. This avoids a compiler warning about a potentially uninitialized variable. Reported-by: kernel test robot <lkp(a)intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/ Signed-off-by: abdelnasser hussein <abdelnasserhussein11(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index 720aa752e17e..6daa4e706792 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -311,8 +311,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec, } /* parse dai_id from AIF widget's stream_name */ - ret = sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir); - if (ret < 3) { + if (sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir) != 3) { dev_err(codec->dev, "Error while parsing dai_id for %s\n", w->name); return -EINVAL; } @@ -323,7 +322,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec, ret = gbaudio_module_enable_tx(codec, module, dai_id); else ret = gbaudio_module_disable_tx(module, dai_id); - } else if (w->id == snd_soc_dapm_aif_out) { + } else { if (enable) ret = gbaudio_module_enable_rx(codec, module, dai_id); else -- 2.54.0

6 days

[PATCH v3 0/2] staging: greybus: audio: cleanups for gbaudio_module_update

by Abdelnasser Hussein

This patch series addresses two separate issues in gbaudio_module_update() that were previously combined in v2: 1. Fixes an improper check of the sscanf() return value (smatch warning). 2. Removes a redundant else-if check that could lead to an uninitialized variable warning for 'ret' (reported by kernel test robot). Changes in v3: - Split the changes into a 2-patch series based on feedback from Greg Kroah-Hartman. - Assigned correct Reported-by and Closes tags to both patches. abdelnasser hussein (2): staging: greybus: audio_codec: fix sscanf return value check staging: greybus: audio_codec: remove redundant else-if check drivers/staging/greybus/audio_codec.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.54.0

6 days

[PATCH] staging: greybus: audio: evaluate sscanf() return value directly

by abdelnasser hussein

Smatch warns: drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update() warn: sscanf doesn't return error codes sscanf() returns the number of successfully matched input items, not a negative error code. Do not store its return value in ret. Check it directly instead. Signed-off-by: abdelnasser hussein <abdelnasserhussein11(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index 720aa752e17e..295222ec0f1a 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -311,8 +311,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec, } /* parse dai_id from AIF widget's stream_name */ - ret = sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir); - if (ret < 3) { + if (sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir) != 3) { dev_err(codec->dev, "Error while parsing dai_id for %s\n", w->name); return -EINVAL; } -- 2.52.0

1 week

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev June 2026