[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/2] tools: use memcpy instead of strncpy in getBridge


  • To: Jürgen Groß <jgross@xxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 7 Oct 2020 08:56:36 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=wVAFq+QOJ1tkIEAjC9X3y8rUyU+ebC1a1YakVtFN3FI=; b=Nx90mLlpZ7frrZm9nDncK0ksoQ53SikFb8yf3itkSMkAIkJKIWhbSC71L85xbj4QylTGwEJUvXcMGr0jpwhYTc7NnJYWurE3k0XqvPbj2E+5vmXMwKGSZwhuocNZ26+aFAY56wnLwixR7M60O6T3l6vKy/NRW7eqDpYOQm2Hobel3Aur8KaSakRzaUvk9DUpfEeSUmR2kyKUobrRxzYZukaJmqSZWTBUcK93gOdfOfiPdR8bbqTlAsGGl8i/QCi93hWs3RMTaTirFOoRxG//H7JcC9BrEvvXD60+wFdEFr/4XUZihFIUWcTAgdFUh7k6+PPhq6TX5enSObEcQAaXOQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KGPoKwhlNg+t38j564yB5vgfDSOSVcoRPkQkwLDAqZUAt1UUxh68b6g4G23XRgfpzEehR3VUQ0sIQAiWC4cfyPzqM6o6wTgZPDAOHhbqe7mhIFKsPyuOyn1rHGAjlbLz5HvRCbqus6uO+A5KkOl0YI/7I+wpcz8UtIByqGdA7ikH4JzAkPd9hQcVjUBYFh83huf2X/+Qx8BXSh5Gq5/veR2o5FpVO96uiO4PQFcQFN/gaXAtvu6WsjCfZ+NoPXejUKe2s3DPu9qhCQgjzwgtUqWv54kywFmFVyWolcolUiN4RGNuywhYj92s9G3sP5Q8maMQmJoI4glsPxVWVQ9mpA==
  • Authentication-results-original: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Wed, 07 Oct 2020 08:56:55 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHWnIQKr7DQ0C+zLEGkNYhGKt2Q8KmL0a8AgAAEzgA=
  • Thread-topic: [PATCH v2 1/2] tools: use memcpy instead of strncpy in getBridge

Hi Jurgen,

> On 7 Oct 2020, at 09:39, Jürgen Groß <jgross@xxxxxxxx> wrote:
> 
> On 07.10.20 10:28, Bertrand Marquis wrote:
>> Use memcpy in getBridge to prevent gcc warnings about truncated
>> strings. We know that we might truncate it, so the gcc warning
>> here is wrong.
>> Revert previous change changing buffer sizes as bigger buffers
>> are not needed.
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> ---
>> Changes in v2:
>>  Use MIN between string length of de->d_name and resultLen to copy only
>>  the minimum size required and prevent crossing to from an unallocated
>>  space.
>> ---
>>  tools/libs/stat/xenstat_linux.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>> diff --git a/tools/libs/stat/xenstat_linux.c 
>> b/tools/libs/stat/xenstat_linux.c
>> index d2ee6fda64..0ace03af1b 100644
>> --- a/tools/libs/stat/xenstat_linux.c
>> +++ b/tools/libs/stat/xenstat_linux.c
>> @@ -29,6 +29,7 @@
>>  #include <string.h>
>>  #include <unistd.h>
>>  #include <regex.h>
>> +#include <xen-tools/libs.h>
>>    #include "xenstat_priv.h"
>>  @@ -78,7 +79,13 @@ static void getBridge(char *excludeName, char *result, 
>> size_t resultLen)
>>                              sprintf(tmp, "/sys/class/net/%s/bridge", 
>> de->d_name);
>>                              if (access(tmp, F_OK) == 0) {
>> -                                    strncpy(result, de->d_name, resultLen);
>> +                                    /*
>> +                                     * Do not use strncpy to prevent 
>> compiler warning with
>> +                                     * gcc >= 10.0
>> +                                     * If de->d_name is longer then 
>> resultLen we truncate it
> 
> s/then/than/

Will fix

> 
>> +                                     */
>> +                                    memcpy(result, de->d_name, 
>> MIN(strnlen(de->d_name,
>> +                                                                    
>> sizeof(de->d_name)),resultLen - 1));
> 
> You can't use sizeof(de->d_name) here, as AFAIK there is no guarantee
> that de->d_name isn't e.g. defined like "char d_name[]".
> 
> My suggestion to use NAME_MAX as upper boundary for the length was
> really meant to be used that way.
> 
> And additionally you might want to add 1 to the strnlen() result in
> order to copy the trailing 0-byte, too (or you should zero out the
> result buffer before and omit writing the final zero byte).
> 
> Thinking more about it zeroing the result buffer is better as it even
> covers the theoretical case of NAME_MAX being shorter than resultLen.

Setting the result buffer completely to 0 and doing after a copy sounds like
a big complexity.

How about:
copysize = MIN(strnlen(de->d_name,NAME_MAX), resultLen - 1);
memcpy(result, de->d_name, copysize);
result[copysize + 1] = 0

This would cover the case where NAME_MAX is shorter then resultLen.

Cheers
Bertrand

> 
> 
> Juergen


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.