JP Kobryn wrote on Mon, Dec 04, 2023 at 12:23:20PM -0800:
> An out of bounds read can occur within the tracepoint 9p_protocol_dump. In
> the fast assign, there is a memcpy that uses a constant size of 32 (macro
> named P9_PROTO_DUMP_SZ). When the copy is invoked, the source buffer is not
> guaranteed match this size. It was found that in some cases the source
> buffer size is less than 32, resulting in a read that overruns.
>
> The size of the source buffer seems to be known at the time of the
> tracepoint being invoked. The allocations happen within p9_fcall_init(),
> where the capacity field is set to the allocated size of the payload
> buffer. This patch tries to fix the overrun by changing the fixed array to
> a dynamically sized array and using the minimum of the capacity value or
> P9_PROTO_DUMP_SZ as its length. The trace log statement is adjusted to
> account for this. Note that the trace log no longer splits the payload on
> the first 16 bytes. The full payload is now logged to a single line.
>
> To repro the orignal problem, operations to a plan 9 managed resource can
> be used. The simplest approach might just be mounting a shared filesystem
> (between host and guest vm) using the plan 9 protocol while the tracepoint
> is enabled.
>
> mount -t 9p -o trans=virtio <mount_tag> <mount_path>
>
> The bpftrace program below can be used to show the out of bounds read.
> Note that a recent version of bpftrace is needed for the raw tracepoint
> support. The script was tested using v0.19.0.
>
> /* from include/net/9p/9p.h */
> struct p9_fcall {
> u32 size;
> u8 id;
> u16 tag;
> size_t offset;
> size_t capacity;
> struct kmem_cache *cache;
> u8 *sdata;
> bool zc;
> };
>
> tracepoint:9p:9p_protocol_dump
> {
> /* out of bounds read can happen when this tracepoint is enabled */
> }
>
> rawtracepoint:9p_protocol_dump
> {
> $pdu = (struct p9_fcall *)arg1;
> $dump_sz = (uint64)32;
>
> if ($dump_sz > $pdu->capacity) {
> printf("reading %zu bytes from src buffer of %zu bytes\n",
> $dump_sz, $pdu->capacity);
> }
> }
>
> Signed-off-by: JP Kobryn <inwardvessel@xxxxxxxxx>
Thanks, I've updated the patch locally; will push to -next after testing
later tonight and to Linus next week.
--
Dominique Martinet | Asmadeus
