linux-kernel - Re: [GIT PULL] tracing: Add __string_len() and __assign_str

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wj5Pp5J-CAPck22RSQ13k3cEOVnJHUA-WocAZqCJK1BZw@mail.gmail.com>
Date:   Wed, 14 Jul 2021 12:20:47 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Chuck Lever <chuck.lever@...cle.com>
Subject: Re: [GIT PULL] tracing: Add __string_len() and __assign_str_len() helpers

On Tue, Jul 13, 2021 at 2:11 PM Steven Rostedt <rostedt@...dmis.org> 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