linux-kernel - Re: [PATCH net-next] tracing: ipv6: Replace deprecated strcpy() with strscpy()

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: <20250723143625.79ab2c16@batman.local.home>
Date: Wed, 23 Jul 2025 14:36:25 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Guillaume Nault <gnault@...hat.com>,
 Paolo Abeni <pabeni@...hat.com>, Ido Schimmel <idosch@...dia.com>, Petr
 Machata <petrm@...dia.com>, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] tracing: ipv6: Replace deprecated strcpy()
 with strscpy()

On Wed, 23 Jul 2025 10:46:12 -0700
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> Your commit fca8300f68fe3 changed it from __dynamic_array() to __array()
> and __string() seems to be just a special version of __dynamic_array()
> with a length of -1.
> 
> In the commit description you wrote: "Since the size of the name is at
> most 16 bytes (defined by IFNAMSIZ), it is not worth spending the effort
> to determine the size of the string."

So the original had:

	__dynamic_array(char,  name,   IFNAMSIZ )

Which is not dynamic at all. A dynamic_array (like __string) saves the
size in meta data within the event. So basically the above is wasting
bytes to save a fixed size. If you are going to use a dynamic array,
might as well make it dynamic!

I was doing various clean ups back then so I didn't look too deeply
into this event when I made that change. I just saw the obvious waste
of space in the ring buffer.

Just to explain it in more detail. A dynamic_array has in the ring buffer:

	short offset;
	short len;
	[..]
	char  name[len];

That is, 4 bytes are used to know the size of the array and where in
the event it is located. Thus the __dynamic_array() usage basically had:

	short offset;
	short len = IFNAMSIZ;
	[..]
	char name[IFNAMSIZ];

Why have the offset and length? with just __array(char, name, IFNAMSIZ}
it would be just:

	char name[IFNAMSIZ];

See why I changed it?

Now, the change I'm suggesting now would make the __string() be dynamic!

	short offset;
	short len = strlen(res->nh && res->nh->fib_nh_dev ? res->nh->fib_nh_dev->name : "-") + 1;
	[..]
	char name[len];

As IFNAMSIZ is 16, and the above adds 4 bytes to the name, if the name
is less than 7 bytes or less, you save memory on the ring buffer.

	2 bytes: offset
	2 bytes: len;
	7 bytes + '\0'

total: 12 bytes

Note, if there's only one dynamic value, it is always at least 4 bytes aligned.

-- Steve