Re: [PATCH] tracing: Remove precision vsnprintf() check from print event

[Mathieu Desnoyers]

On 2024-03-04 17:43, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> This reverts 60be76eeabb3d ("tracing: Add size check when printing
> trace_marker output"). The only reason the precision check was added
> was because of a bug that miscalculated the write size of the string into
> the ring buffer and it truncated it removing the terminating nul byte. On
> reading the trace it crashed the kernel. But this was due to the bug in
> the code that happened during development and should never happen in
> practice. If anything, the precision can hide bugs where the string in the
> ring buffer isn't nul terminated and it will not be checked.
>
> Link: https://lore.kernel.org/all/C7E7AF1A-D30F-4D18-B8E5-AF1EF58004F5@xxxxxxxxxxxxx/
> Link: https://lore.kernel.org/linux-trace-kernel/20240227125706.04279ac2@xxxxxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/all/20240302111244.3a1674be@xxxxxxxxxxxxxxxxxx/
>
> Reported-by: Sachin Sant <sachinp@xxxxxxxxxxxxx>
> Fixes: 60be76eeabb3d ("tracing: Add size check when printing trace_marker output")
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>

This is a step in the right direction IMHO.

Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

Just out of curiosity, is there anything to prevent trace_marker from
writing a huge string into the ring buffer in the first place ? Is this
limit implicit and based on the page size of the architecture or is it
a known fixed limit ? (e.g. 4kB strings).

It appears to currently be limited by

#define TRACE_SEQ_BUFFER_SIZE   (PAGE_SIZE * 2 - \
        (sizeof(struct seq_buf) + sizeof(size_t) + sizeof(int)))

checked within tracing_mark_write().

I would have hoped for a simpler limit (e.g. 4kB) consistent across
architectures. But that would belong to a separate change.

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com