Re: [PATCH v1 3/4] tracing/remotes: Add REMOTE_EVENT_CUSTOM_PRINTK() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing payload length validation and broken event pointer arithmetic causing Out-of-Bounds reads.
--

commit 08a76348182599b6627f69fa9a3f00bf4080d17a
Author: Vincent Donnefort <vdonnefort@google.com>

tracing/remotes: Add REMOTE_EVENT_CUSTOM_PRINTK() helper

This commit introduces a REMOTE_EVENT_CUSTOM_PRINTK() macro where the
__printk argument can be a function. This intends to support the creation
of a dynamic "printk" event for the arm64 nVHE/pKVM hypervisor.

> diff --git a/include/trace/define_remote_events.h b/include/trace/define_remote_events.h
> index 676e803dc144d..4f4d58e37b84b 100644
> --- a/include/trace/define_remote_events.h
> +++ b/include/trace/define_remote_events.h
[ ... ]
> +#define REMOTE_EVENT_PRINT_FUNC(__name, __printk)					\
>  	static void remote_event_print_##__name(void *evt, struct trace_seq *seq)	\
>  	{										\
>  		struct remote_event_format_##__name __maybe_unused *__entry = evt;	\

[Severity: High]
Does this direct cast of the evt payload pointer lack length validation?

It appears there is no check to ensure the event length from the ring
buffer is large enough to hold the format structure before casting.

Additionally, in the final state of the series, is there an issue
calculating the number of arguments in __hyp_trace_printk()?

arch/arm64/kvm/hyp_trace.c:__hyp_trace_printk() {
    struct ring_buffer_event *evt = (void *)entry - RB_EVNT_HDR_SIZE;
    ...
    nr_args = (ring_buffer_event_length(evt) - offsetof(struct remote_event_format_hyp_printk, args)) / sizeof(entry->args[0]);
}

Because the hypervisor uses simple_ring_buffer, it writes events with
type_len = 0 (indicating the length is stored in array[0]). Consequently,
ring_buffer_event_data() skips the 4-byte length field, meaning the entry
payload pointer actually points to &rb_event->array[1].

Subtracting RB_EVNT_HDR_SIZE (4 bytes) yields a pointer to
&rb_event->array[0], not the actual event header. 

When ring_buffer_event_length(evt) is called on this misaligned pointer,
could it interpret the original length or the payload's fmt_id field
as the type_len header?

This would lead to highly incorrect length calculations (e.g., treating
an event size of 20 as 64 bytes), guaranteeing that __hyp_trace_printk
will either silently drop arguments or read arguments out-of-bounds. 

If the event is near the end of a mapped page, could reading past
these boundaries trigger a host kernel panic?

>  		trace_seq_puts(seq, #__name);						\
> -		remote_printk(__printk);						\
> +		__printk;								\
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612142245.1015744-1-vdonnefort@google.com?part=3