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

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

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@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@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@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@proton.me>

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

[Bryam Vargas]

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@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@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@proton.me>

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

[Greg Kroah-Hartman]

On Tue, Jun 16, 2026 at 01:06:12AM -0500, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@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.

But we trust the hardware to send us proper data, right?  If we don't
trust modules, then there are lots of other places stuff like this needs
to be fixed, how many data paths did you audit?

> 
> 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@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@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.


How did you find/fix this?  You need to list what tools helped you...

> ---
>  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)) {

Are you sure these checks will not overflow?

thanks,

greg k-h

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

[Dan Carpenter]

On Tue, Jun 16, 2026 at 12:01:30PM +0530, Greg Kroah-Hartman wrote:
> On Tue, Jun 16, 2026 at 01:06:12AM -0500, Bryam Vargas via B4 Relay wrote:
> > ---
> >  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)) {
> 
> Are you sure these checks will not overflow?


Yep.  The cast to u64 ensures that.

regards,
dan carpenter

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

[Bryam Vargas]

Hi Greg, and thanks Dan,

>> Are you sure these checks will not overflow?
> Yep.  The cast to u64 ensures that.

Right, and to close the other side of the comparison too: `size` is a u16 and
the function already does `if (size < sizeof(*topo)) return -ENODATA;` above
this point, so `size - sizeof(*topo)` cannot underflow either. The left side is
the (u64) sum of four u32s (max ~2^34), so neither side wraps. The form
`sizeof(*topo) + sum > size` is exactly equivalent if it reads more clearly.

> But we trust the hardware to send us proper data, right?  If we don't trust
> modules, then there are lots of other places stuff like this needs to be
> fixed, how many data paths did you audit?

I audited the four size_* fields that gbaudio_tplg_parse_data() turns into
section offsets -- those are the only module-supplied values that feed directly
into unchecked pointer arithmetic (control/widget/route_offset are dai_offset
plus those le32s, then dereferenced as structs). I am not claiming a broader
greybus or topology-parser audit; that is welcome but separate.

It is less "modules are malicious" than "a malformed or buggy module response
should not walk the parser off a slab object" -- the same
untrusted-length-to-offset shape already hardened for USB/HID/BT descriptors.
If you would rather treat module data as trusted and drop the stable tag, that
is your call; I would keep the bound regardless, since it is one branch and the
offsets are otherwise completely unchecked.

> How did you find/fix this?  You need to list what tools helped you...

I do not have real greybus audio hardware, so I simulated the module side and
drove the negative case directly: a topology whose fetched `size` is small but
whose size_* fields are large -- exactly the invariant this patch enforces.
With that I reproduced the read two ways:

  - in-kernel under KASAN (7.1.0-rc5): slab-out-of-bounds 4 bytes past a
    kmalloc-64 object; the patched arm (-EINVAL) and an in-bounds arm are clean;
  - a userspace AddressSanitizer model of the process_header() offset walk,
    both -m32 and -m64.

Tools: a static read of the audio_gb.c -> audio_topology.c data flow, a litmus
greybus module under KASAN in a VM, and the userspace ASan harness. The
verifiable artifact is the KASAN splat (trimmed under the --- in the original
posting; full log on request).

Thanks,
Bryam