linux-kernel - Re: [GIT PULL] tracing: histogram fix and take 2 on the __string

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: <20210716143705.56001390@oasis.local.home>
Date:   Fri, 16 Jul 2021 14:37:05 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.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: histogram fix and take 2 on the
 __string_len() marcros

On Fri, 16 Jul 2021 11:11:38 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Thu, Jul 15, 2021 at 6:57 PM Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > tracing: One fix in the histogram code and another take at the __string_len() macro  
> 
> What part of "strncpy()" is garbage did I not make clear?

So how do you want this implemented?

#define __assign_str_len(dst, src, len)					\
	do {								\
		strscpy(__get_str(dst), (src) ? (const char *)(src) : "(null)", len); \
		__get_str(dst)[len] = '\0';				\
	} while(0)

I could even do:

#define __assign_str_len(dst, src, len)					\
	do {								\
		if (!src && len > 6)					\
			len = 6;					\
		memcpy(__get_str(dst), (src) ? (const char *)(src) : "(null)", len); \
		__get_str(dst)[len] = '\0';				\
	} while(0)

Which would work just as well, although I had to account if len is
greater than "(null)". Which should never happen, but I have it there,
because I copied the code from the __string() version which uses
strlen() and that would break if a null is passed in (which in rare
cases happen). But it would actually be a bug to use __string_len() in
a place that can take a NULL, unless len was zero.

Not sure how the above is any different than using strncpy().

Again, src is a string without a '\0'. What I don't understand is how
strscpy() is any better than strncpy() in this situation?

As I replied to you last time, the dst is created by allocating 'len +
1' on the ring buffer, and len is to be no greater than the number of
characters in src.

The only purpose to use this is to either truncate a string, or to pass
in a string that was read from a memory location that does not have a
terminating '\0' in it, but you know the length of the string. If you
have a normal string, simply use the __string() which uses strlen() to
calculate the required space.

It's basically this:

	dst = malloc(len + 1);
	memcpy(dst, src, len);
	dst[len] = '\0';

"strncpy() is garbage" does not compute to me in the above usage.

-- Steve