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  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]
Date:   Tue, 19 Oct 2021 12:30:28 -0500
From:   "Zanussi, Tom" <>
To:     Steven Rostedt <>,
        Yang Jihong <>
Cc:     linux-kernel <>
Subject: Re: [QUESTION] Performance deterioration caused by commit

Hi Steve,

On 10/18/2021 9:51 PM, Steven Rostedt wrote:
> On Tue, 19 Oct 2021 10:39:47 +0800
> Yang Jihong <> wrote:
>> Hi Steve,
>> On 2021/10/18 21:37, Steven Rostedt wrote:
>>> On Mon, 18 Oct 2021 11:23:14 +0800
>>> Yang Jihong <> wrote:
>>>> Hi Tom and Steven,
>>>> commit 85f726a35e504418 use strncpy instead of memcpy when copying comm,
>>>> on ARM64 machine, this commit causes performance degradation.
>>>> I test the number of instructions executed by invoking the
>>>> trace_sched_switch function once on an arm64 machine:
>>>> 1. Use memcpy, the number of instructions executed is 850.
>>>> 2. Use strncpy, the number of instructions executed 1100.
>>>> That is, use strncpy is almost 250 more instructions than memcpy.
>>>> Has the impact on performance been considered in this commit? :)
>>>> What is the impact of revert the patch?
>>> It's a security issue. And like everything security, there's always going
>>> to be a performance impact. Look at the performance impact due to spectre
>>> and meltdown!
>>> That said, although memcpy() may not be used, we don't need strncpy.
>>> strncpy() will pad the rest of the string with nul bytes. But since the
>>> memory the string is being recorded into is already initialized (or can be
>>> if it isn't), we could use the faster strlcpy().
>>> Have you tried testing it by switching strncpy() with strlcpy()?
>> I have tried testing it by switching strncpy() with strlcpy(), there is
>> no performance improvement, probably because the strlen function is
>> called in strlpy and the string is traversed each time.
> Then there's not much we can do. Security trumps performance. Not to
> mention, the garbage in the comm after the '\0' causes the histograms to
> produce strange results.
> Now for the saved_cmdlines, since it isn't exported directly to user space,
> that one may be put back to memcpy().
> Tom, was there a reason to change saved_cmdlines(), as I'm not sure that is
> leaked. It looks like it is printed with the normal seq_printf() in
> saved_cmdlines_show().

I don't think either of the changes in commit 85f726a35e504418 are directly related to the original problem [1] and therefore changing them back to memcpy or whatever shouldn't affect the histograms since that data is never used in keys.

Commit 85f726a35e504418 was basically a follow-on to commit 9f0bbf3115ca (tracing: Use strncpy instead of memcpy for string keys in hist triggers) and was added for completeness after examining other uses of memcpy in the tracing code (there's even a comment in there from you about possible performance hits from changing it ;-)

So anyway, as far as the histograms go, I think optimizing the two changes in 85f726a35e504418 while ignoring trailing garbage can be done without affecting the histogram correctness.



> And it doesn't even look like the saved_cmdlines() is even initialized to
> zero, so it itself could leak memory if it was exposed.
> -- Steve

Powered by blists - more mailing lists