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

Bertrand Marquis posted 2 patches 1 month, 3 weeks ago
Maintainers: Anthony PERARD <anthony.perard@citrix.com>, Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>

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

Posted by Bertrand Marquis 1 month, 3 weeks ago
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@arm.com>
---
 tools/libs/stat/xenstat_linux.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
index d2ee6fda64..1db35c604c 100644
--- a/tools/libs/stat/xenstat_linux.c
+++ b/tools/libs/stat/xenstat_linux.c
@@ -78,7 +78,12 @@ 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
+					 */
+					memcpy(result, de->d_name, resultLen - 1);
 					result[resultLen - 1] = 0;
 				}
 		}
@@ -264,7 +269,7 @@ int xenstat_collect_networks(xenstat_node * node)
 {
 	/* Helper variables for parseNetDevLine() function defined above */
 	int i;
-	char line[512] = { 0 }, iface[16] = { 0 }, devBridge[256] = { 0 }, devNoBridge[257] = { 0 };
+	char line[512] = { 0 }, iface[16] = { 0 }, devBridge[16] = { 0 }, devNoBridge[17] = { 0 };
 	unsigned long long rxBytes, rxPackets, rxErrs, rxDrops, txBytes, txPackets, txErrs, txDrops;
 
 	struct priv_data *priv = get_priv_data(node->handle);
-- 
2.17.1


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

Posted by Jürgen Groß 1 month, 3 weeks ago
On 05.10.20 18:02, 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@arm.com>
> ---
>   tools/libs/stat/xenstat_linux.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
> index d2ee6fda64..1db35c604c 100644
> --- a/tools/libs/stat/xenstat_linux.c
> +++ b/tools/libs/stat/xenstat_linux.c
> @@ -78,7 +78,12 @@ 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
> +					 */
> +					memcpy(result, de->d_name, resultLen - 1);

I think you want min(NAME_MAX, resultLen - 1) for the length.


Juergen

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

Posted by Bertrand Marquis 1 month, 3 weeks ago

> On 6 Oct 2020, at 05:34, Jürgen Groß <jgross@suse.com> wrote:
> 
> On 05.10.20 18:02, 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@arm.com>
>> ---
>>  tools/libs/stat/xenstat_linux.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
>> index d2ee6fda64..1db35c604c 100644
>> --- a/tools/libs/stat/xenstat_linux.c
>> +++ b/tools/libs/stat/xenstat_linux.c
>> @@ -78,7 +78,12 @@ 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
>> +					 */
>> +					memcpy(result, de->d_name, resultLen - 1);
> 
> I think you want min(NAME_MAX, resultLen - 1) for the length.

true, I will fix that and send a v2.

Cheers
Bertrand

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

Posted by Jürgen Groß 1 month, 3 weeks ago
On 06.10.20 09:51, Bertrand Marquis wrote:
> 
> 
>> On 6 Oct 2020, at 05:34, Jürgen Groß <jgross@suse.com> wrote:
>>
>> On 05.10.20 18:02, 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@arm.com>
>>> ---
>>>   tools/libs/stat/xenstat_linux.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
>>> index d2ee6fda64..1db35c604c 100644
>>> --- a/tools/libs/stat/xenstat_linux.c
>>> +++ b/tools/libs/stat/xenstat_linux.c
>>> @@ -78,7 +78,12 @@ 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
>>> +					 */
>>> +					memcpy(result, de->d_name, resultLen - 1);
>>
>> I think you want min(NAME_MAX, resultLen - 1) for the length.
> 
> true, I will fix that and send a v2.

Hmm, maybe you should use

min(strnlen(de->d_name, NAME_MAX), resultLen - 1)

for the case that de->d_name is near the end of a page, as otherwise
you could try to copy unallocated space.


Juergen

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

Posted by Bertrand Marquis 1 month, 3 weeks ago
Hi Jurgen,

> On 6 Oct 2020, at 09:19, Jürgen Groß <jgross@suse.com> wrote:
> 
> On 06.10.20 09:51, Bertrand Marquis wrote:
>>> On 6 Oct 2020, at 05:34, Jürgen Groß <jgross@suse.com> wrote:
>>> 
>>> On 05.10.20 18:02, 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@arm.com>
>>>> ---
>>>>  tools/libs/stat/xenstat_linux.c | 9 +++++++--
>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
>>>> index d2ee6fda64..1db35c604c 100644
>>>> --- a/tools/libs/stat/xenstat_linux.c
>>>> +++ b/tools/libs/stat/xenstat_linux.c
>>>> @@ -78,7 +78,12 @@ 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
>>>> +					 */
>>>> +					memcpy(result, de->d_name, resultLen - 1);
>>> 
>>> I think you want min(NAME_MAX, resultLen - 1) for the length.
>> true, I will fix that and send a v2.
> 
> Hmm, maybe you should use
> 
> min(strnlen(de->d_name, NAME_MAX), resultLen - 1)
> 
> for the case that de->d_name is near the end of a page, as otherwise
> you could try to copy unallocated space.
> 

Agree, I will do that.

Cheers
Bertrand

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

Posted by Wei Liu 1 month, 3 weeks ago
On Mon, Oct 05, 2020 at 05:02:48PM +0100, 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@arm.com>

Juergen, are you happy with this change? I have not followed closely the
discussion on #xendevel.

> ---
>  tools/libs/stat/xenstat_linux.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
> index d2ee6fda64..1db35c604c 100644
> --- a/tools/libs/stat/xenstat_linux.c
> +++ b/tools/libs/stat/xenstat_linux.c
> @@ -78,7 +78,12 @@ 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
> +					 */
> +					memcpy(result, de->d_name, resultLen - 1);
>  					result[resultLen - 1] = 0;
>  				}
>  		}
> @@ -264,7 +269,7 @@ int xenstat_collect_networks(xenstat_node * node)
>  {
>  	/* Helper variables for parseNetDevLine() function defined above */
>  	int i;
> -	char line[512] = { 0 }, iface[16] = { 0 }, devBridge[256] = { 0 }, devNoBridge[257] = { 0 };
> +	char line[512] = { 0 }, iface[16] = { 0 }, devBridge[16] = { 0 }, devNoBridge[17] = { 0 };
>  	unsigned long long rxBytes, rxPackets, rxErrs, rxDrops, txBytes, txPackets, txErrs, txDrops;
>  
>  	struct priv_data *priv = get_priv_data(node->handle);
> -- 
> 2.17.1
>