linux-hardening - Re: [RFC PATCH] VMCI: Silence memcpy() run-time false positive warning

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 4 Jan 2024 19:31:26 +0100
From: Vegard Nossum <vegard.nossum@...cle.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
Cc: linux-hardening@...r.kernel.org, keescook@...omium.org,
        gustavoars@...nel.org, Bryan Tan <bryantan@...are.com>,
        Vishnu Dasa <vdasa@...are.com>,
        VMware PV-Drivers Reviewers <pv-drivers@...are.com>,
        Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
        darren.kenny@...cle.com, syzkaller <syzkaller@...glegroups.com>
Subject: Re: [RFC PATCH] VMCI: Silence memcpy() run-time false positive
 warning


On 01/01/2024 14:55, Greg Kroah-Hartman wrote:
> On Mon, Jan 01, 2024 at 05:08:28AM -0800, Harshit Mogalapalli wrote:
>> One possible way to silence the warning is to split the memcpy() into
>> two parts -- one -- copying the msg and second taking care of payload.
> 
> And what are the performance impacts of this?

I did a disasssembly diff for the version of the patch that uses
dg->payload_size directly in the second memcpy and I get this as the
only change:

@@ -419,11 +419,16 @@
         mov    %rax,%rbx
         test   %rax,%rax
         je
+       mov    0x0(%rbp),%rdx
         mov    %r14,(%rax)
-       mov    %r13,%rdx
-       mov    %rbp,%rsi
-       lea    0x30(%rax),%rdi
+       lea    0x18(%rbp),%rsi
+       lea    0x48(%rax),%rdi
         movb   $0x1,0x28(%rax)
+       mov    %rdx,0x30(%rax)
+       mov    0x8(%rbp),%rdx
+       mov    %rdx,0x38(%rax)
+       mov    0x10(%rbp),%rdx
+       mov    %rdx,0x40(%rax)
         call
         mov    0x0(%rip),%rsi        #
         lea    0x8(%rbx),%rdx

Basically, I believe it's inlining the first constant-size memcpy and
keeping the second one as a call.

Overall, the number of memory accesses should be the same.

The biggest impact that I can see is therefore the code size (which
isn't much).

There is also a kmalloc() on the same code path that I assume would
dwarf any performance impact from this patch -- but happy to be corrected.


Vegard