In the case that 'extra' isn't a multiple of uint32_t, the calculation rounds the number of bytes up, causing later logic to read unrelated bytes beyond the end of the object.
Also, asserting that the object is within TRACE_EXTRA_MAX, but truncating it in release builds is rude. Instead, reject any out-of-spec records, leaving enough of a message to identify the faulty caller. Signed-off-by: Andrew Cooper <[email protected]> --- CC: George Dunlap <[email protected]> CC: Ian Jackson <[email protected]> CC: Jan Beulich <[email protected]> CC: Stefano Stabellini <[email protected]> CC: Wei Liu <[email protected]> CC: Julien Grall <[email protected]> CC: Dario Faggioli <[email protected]> I've eyeballed the code and can't spot any problematic callers, but I came very close to accidentally introducing some when trying to fix the stack rubble leaks in subsequent patches. --- xen/common/trace.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/xen/common/trace.c b/xen/common/trace.c index a2a389a1c7c3..25af6e1bd25e 100644 --- a/xen/common/trace.c +++ b/xen/common/trace.c @@ -686,22 +686,21 @@ void __trace_var(u32 event, bool_t cycles, unsigned int extra, unsigned long flags; u32 bytes_to_tail, bytes_to_wrap; unsigned int rec_size, total_size; - unsigned int extra_word; bool_t started_below_highwater; if( !tb_init_done ) return; - /* Convert byte count into word count, rounding up */ - extra_word = (extra / sizeof(u32)); - if ( (extra % sizeof(u32)) != 0 ) - extra_word++; - - ASSERT(extra_word <= TRACE_EXTRA_MAX); - extra_word = min_t(int, extra_word, TRACE_EXTRA_MAX); - - /* Round size up to nearest word */ - extra = extra_word * sizeof(u32); + /* + * Trace records require extra data which is an exact multiple of + * uint32_t. Reject out-of-spec records. Any failure here is an error in + * the caller. + */ + if ( extra % sizeof(uint32_t) || + extra / sizeof(uint32_t) > TRACE_EXTRA_MAX ) + return printk_once(XENLOG_WARNING + "Trace event %#x bad size %u, discarding\n", + event, extra); if ( (tb_event_mask & event) == 0 ) return; -- 2.11.0
