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/