Re: [GIT PULL] tracing: Add __string_len() and __assign_str_len() helpers

[Linus Torvalds]

On Tue, Jul 13, 2021 at 2:11 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> Add macros for the TRACE_EVENT() macro that can be used to assign strings
> that either need to be truncated, or have no nul terminator, and depends
> on a length attribute to assign.

I pulled this, but then I looked at the actual patch, and decided it's
not acceptable.

> +#define __assign_str_len(dst, src, len)                                                \
> +       strncpy(__get_str(dst), (src) ? (const char *)(src) : "(null)", len);   \
> +       __get_str(dst)[len] = '\0';

I can see so many problems in the above that it's not even funny.

Maybe all users would end up avoiding the pitfalls, but the above
really is disgusting.

And yes, there's a pre-existing multi-statement macro without any
grouping, but that's not an excuse for doing more of them, and doing
them badly.

And by "badly" I mean - among other things - the questionable NUL
termination that *overflows* the size that was specified, but also
using strncpy() at all.

Hint: use strscpy instead of re-implementing it badly. If you really
want the crazy NUL padding that strncpy does - which I doubt you do -
use strscpy_pad(), making it explicit.

             Linus