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