Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210713171143.7784697e@oasis.local.home>
In-Reply-To: <20210713171143.7784697e@oasis.local.home>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 14 Jul 2021 12:20:47 -0700
Message-ID: <CAHk-=wj5Pp5J-CAPck22RSQ13k3cEOVnJHUA-WocAZqCJK1BZw@mail.gmail.com>
Subject: Re: [GIT PULL] tracing: Add __string_len() and __assign_str_len() helpers
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Chuck Lever <chuck.lever@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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