Received-SPF: pass (google.com: domain of linux-kernel+bounces-17119-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <c9f029c5-713b-4500-85f1-a72dec5c3909@embeddedor.com>
Date: Thu, 4 Jan 2024 13:02:33 -0600
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH] VMCI: Silence memcpy() run-time false positive
 warning
Content-Language: en-US
To: Vegard Nossum <vegard.nossum@oracle.com>,
 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Cc: linux-hardening@vger.kernel.org, keescook@chromium.org,
 gustavoars@kernel.org, Bryan Tan <bryantan@vmware.com>,
 Vishnu Dasa <vdasa@vmware.com>,
 VMware PV-Drivers Reviewers <pv-drivers@vmware.com>,
 Arnd Bergmann <arnd@arndb.de>, linux-kernel@vger.kernel.org,
 darren.kenny@oracle.com, syzkaller <syzkaller@googlegroups.com>
References: <20240101130828.3666251-1-harshit.m.mogalapalli@oracle.com>
 <2024010103-connector-plausibly-bc35@gregkh>
 <c42282c1-238b-4e35-bdac-1eb8260fe351@oracle.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
In-Reply-To: <c42282c1-238b-4e35-bdac-1eb8260fe351@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 1/4/24 12:31, Vegard Nossum wrote:
> 
> 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).

Yep, I don't think this is a problem.

I look forward to reviewing v2 of this patch.

Thanks
--
Gustavo

> 
> 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
>