All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] perf cpumap: Fix snprintf overflow check
@ 2020-03-24  7:03 ` Christophe JAILLET
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe JAILLET @ 2020-03-24  7:03 UTC (permalink / raw)
  To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
	namhyung, kan.liang, zhe.he, dzickus, jstancek
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET

'snprintf' returns the number of characters which would be generated for
the given input.

If the returned value is *greater than* or equal to the buffer size, it
means that the output has been truncated.

Fix the overflow test accordingling.

Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
V2: keep snprintf
    modifiy the tests for truncated output
    Update subject and description
---
 tools/perf/util/cpumap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 983b7388f22b..dc5c5e6fc502 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest present cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -356,7 +356,7 @@ static void set_max_node_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
 		return 0;
 
 	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
-	if (n == PATH_MAX) {
+	if (n >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		return -1;
 	}
@@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
 			continue;
 
 		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
-		if (n == PATH_MAX) {
+		if (n >= PATH_MAX) {
 			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 			continue;
 		}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V2] perf cpumap: Fix snprintf overflow check
@ 2020-03-24  7:03 ` Christophe JAILLET
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe JAILLET @ 2020-03-24  7:03 UTC (permalink / raw)
  To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
	namhyung, kan.liang, zhe.he, dzickus, jstancek
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET

'snprintf' returns the number of characters which would be generated for
the given input.

If the returned value is *greater than* or equal to the buffer size, it
means that the output has been truncated.

Fix the overflow test accordingling.

Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
V2: keep snprintf
    modifiy the tests for truncated output
    Update subject and description
---
 tools/perf/util/cpumap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 983b7388f22b..dc5c5e6fc502 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest present cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -356,7 +356,7 @@ static void set_max_node_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
 		return 0;
 
 	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
-	if (n = PATH_MAX) {
+	if (n >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		return -1;
 	}
@@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
 			continue;
 
 		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
-		if (n = PATH_MAX) {
+		if (n >= PATH_MAX) {
 			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 			continue;
 		}
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
  2020-03-24  7:03 ` Christophe JAILLET
@ 2020-03-24 12:50   ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-24 12:50 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
	kan.liang, zhe.he, dzickus, jstancek, David Laight, linux-kernel,
	kernel-janitors

Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
> 'snprintf' returns the number of characters which would be generated for
> the given input.
> 
> If the returned value is *greater than* or equal to the buffer size, it
> means that the output has been truncated.
> 
> Fix the overflow test accordingling.
                                  y

You forgot to CC David and add this to your patch, which I did:

Suggested-by: David Laight <David.Laight@ACULAB.COM>

Ok?

- Arnaldo
 
> Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
> Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> V2: keep snprintf
>     modifiy the tests for truncated output
>     Update subject and description
> ---
>  tools/perf/util/cpumap.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 983b7388f22b..dc5c5e6fc502 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
>  
>  	/* get the highest possible cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
> -	if (ret == PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
>  
>  	/* get the highest present cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
> -	if (ret == PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -356,7 +356,7 @@ static void set_max_node_num(void)
>  
>  	/* get the highest possible cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
> -	if (ret == PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
>  		return 0;
>  
>  	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
> -	if (n == PATH_MAX) {
> +	if (n >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		return -1;
>  	}
> @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
>  			continue;
>  
>  		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
> -		if (n == PATH_MAX) {
> +		if (n >= PATH_MAX) {
>  			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  			continue;
>  		}
> -- 
> 2.20.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
@ 2020-03-24 12:50   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-24 12:50 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
	kan.liang, zhe.he, dzickus, jstancek, David Laight, linux-kernel,
	kernel-janitors

Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
> 'snprintf' returns the number of characters which would be generated for
> the given input.
> 
> If the returned value is *greater than* or equal to the buffer size, it
> means that the output has been truncated.
> 
> Fix the overflow test accordingling.
                                  y

You forgot to CC David and add this to your patch, which I did:

Suggested-by: David Laight <David.Laight@ACULAB.COM>

Ok?

- Arnaldo
 
> Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
> Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> V2: keep snprintf
>     modifiy the tests for truncated output
>     Update subject and description
> ---
>  tools/perf/util/cpumap.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 983b7388f22b..dc5c5e6fc502 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
>  
>  	/* get the highest possible cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
> -	if (ret = PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
>  
>  	/* get the highest present cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
> -	if (ret = PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -356,7 +356,7 @@ static void set_max_node_num(void)
>  
>  	/* get the highest possible cpu number for a sparse allocation */
>  	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
> -	if (ret = PATH_MAX) {
> +	if (ret >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		goto out;
>  	}
> @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
>  		return 0;
>  
>  	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
> -	if (n = PATH_MAX) {
> +	if (n >= PATH_MAX) {
>  		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  		return -1;
>  	}
> @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
>  			continue;
>  
>  		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
> -		if (n = PATH_MAX) {
> +		if (n >= PATH_MAX) {
>  			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>  			continue;
>  		}
> -- 
> 2.20.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
  2020-03-24 12:50   ` Arnaldo Carvalho de Melo
@ 2020-03-24 14:44     ` Christophe JAILLET
  -1 siblings, 0 replies; 10+ messages in thread
From: Christophe JAILLET @ 2020-03-24 14:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
	kan.liang, zhe.he, dzickus, jstancek, David Laight, linux-kernel,
	kernel-janitors

Le 24/03/2020 à 13:50, Arnaldo Carvalho de Melo a écrit :
> Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
>> 'snprintf' returns the number of characters which would be generated for
>> the given input.
>>
>> If the returned value is *greater than* or equal to the buffer size, it
>> means that the output has been truncated.
>>
>> Fix the overflow test accordingling.
>                                    y
>
> You forgot to CC David and add this to your patch, which I did:
>
> Suggested-by: David Laight <David.Laight@ACULAB.COM>
>
> Ok?

No problem for me.

The to: and cc: list are (at least should be :)) the ones given by 
get_maintainer, and I was not aware of the 'Suggested-by' tag.

BTW, thanks for fixing the typo in the description.

CJ


> - Arnaldo
>   
>> Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
>> Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> V2: keep snprintf
>>      modifiy the tests for truncated output
>>      Update subject and description
>> ---
>>   tools/perf/util/cpumap.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
>> index 983b7388f22b..dc5c5e6fc502 100644
>> --- a/tools/perf/util/cpumap.c
>> +++ b/tools/perf/util/cpumap.c
>> @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
>>   
>>   	/* get the highest possible cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
>> -	if (ret == PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
>>   
>>   	/* get the highest present cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
>> -	if (ret == PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -356,7 +356,7 @@ static void set_max_node_num(void)
>>   
>>   	/* get the highest possible cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
>> -	if (ret == PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
>>   		return 0;
>>   
>>   	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
>> -	if (n == PATH_MAX) {
>> +	if (n >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		return -1;
>>   	}
>> @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
>>   			continue;
>>   
>>   		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
>> -		if (n == PATH_MAX) {
>> +		if (n >= PATH_MAX) {
>>   			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   			continue;
>>   		}
>> -- 
>> 2.20.1
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
@ 2020-03-24 14:44     ` Christophe JAILLET
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe JAILLET @ 2020-03-24 14:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
	kan.liang, zhe.he, dzickus, jstancek, David Laight, linux-kernel,
	kernel-janitors

Le 24/03/2020 à 13:50, Arnaldo Carvalho de Melo a écrit :
> Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
>> 'snprintf' returns the number of characters which would be generated for
>> the given input.
>>
>> If the returned value is *greater than* or equal to the buffer size, it
>> means that the output has been truncated.
>>
>> Fix the overflow test accordingling.
>                                    y
>
> You forgot to CC David and add this to your patch, which I did:
>
> Suggested-by: David Laight <David.Laight@ACULAB.COM>
>
> Ok?

No problem for me.

The to: and cc: list are (at least should be :)) the ones given by 
get_maintainer, and I was not aware of the 'Suggested-by' tag.

BTW, thanks for fixing the typo in the description.

CJ


> - Arnaldo
>   
>> Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
>> Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> V2: keep snprintf
>>      modifiy the tests for truncated output
>>      Update subject and description
>> ---
>>   tools/perf/util/cpumap.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
>> index 983b7388f22b..dc5c5e6fc502 100644
>> --- a/tools/perf/util/cpumap.c
>> +++ b/tools/perf/util/cpumap.c
>> @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
>>   
>>   	/* get the highest possible cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
>> -	if (ret = PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
>>   
>>   	/* get the highest present cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
>> -	if (ret = PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -356,7 +356,7 @@ static void set_max_node_num(void)
>>   
>>   	/* get the highest possible cpu number for a sparse allocation */
>>   	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
>> -	if (ret = PATH_MAX) {
>> +	if (ret >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		goto out;
>>   	}
>> @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
>>   		return 0;
>>   
>>   	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
>> -	if (n = PATH_MAX) {
>> +	if (n >= PATH_MAX) {
>>   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   		return -1;
>>   	}
>> @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
>>   			continue;
>>   
>>   		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
>> -		if (n = PATH_MAX) {
>> +		if (n >= PATH_MAX) {
>>   			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
>>   			continue;
>>   		}
>> -- 
>> 2.20.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
  2020-03-24 14:44     ` Christophe JAILLET
@ 2020-03-25 12:19       ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-25 12:19 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: Arnaldo Carvalho de Melo, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung, kan.liang, zhe.he, dzickus,
	jstancek, David Laight, linux-kernel, kernel-janitors

Em Tue, Mar 24, 2020 at 03:44:15PM +0100, Christophe JAILLET escreveu:
> Le 24/03/2020 à 13:50, Arnaldo Carvalho de Melo a écrit :
> > Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
> > > 'snprintf' returns the number of characters which would be generated for
> > > the given input.
> > > 
> > > If the returned value is *greater than* or equal to the buffer size, it
> > > means that the output has been truncated.
> > > 
> > > Fix the overflow test accordingling.
> >                                    y
> > 
> > You forgot to CC David and add this to your patch, which I did:
> > 
> > Suggested-by: David Laight <David.Laight@ACULAB.COM>
> > 
> > Ok?
> 
> No problem for me.
> 
> The to: and cc: list are (at least should be :)) the ones given by
> get_maintainer, and I was not aware of the 'Suggested-by' tag.

Sure, that is the starting point, but if someone participates in the
discussion and provides suggestions, comments, then it is sensible to
add them to the CC list in follow up patches.
 
> BTW, thanks for fixing the typo in the description.

Part of my job as a maintainer :-)

- Arnaldo
 
> CJ
> 
> 
> > - Arnaldo
> > > Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
> > > Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
> > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > > ---
> > > V2: keep snprintf
> > >      modifiy the tests for truncated output
> > >      Update subject and description
> > > ---
> > >   tools/perf/util/cpumap.c | 10 +++++-----
> > >   1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > > index 983b7388f22b..dc5c5e6fc502 100644
> > > --- a/tools/perf/util/cpumap.c
> > > +++ b/tools/perf/util/cpumap.c
> > > @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
> > >   	/* get the highest possible cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
> > > -	if (ret == PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
> > >   	/* get the highest present cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
> > > -	if (ret == PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -356,7 +356,7 @@ static void set_max_node_num(void)
> > >   	/* get the highest possible cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
> > > -	if (ret == PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
> > >   		return 0;
> > >   	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
> > > -	if (n == PATH_MAX) {
> > > +	if (n >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		return -1;
> > >   	}
> > > @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
> > >   			continue;
> > >   		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
> > > -		if (n == PATH_MAX) {
> > > +		if (n >= PATH_MAX) {
> > >   			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   			continue;
> > >   		}
> > > -- 
> > > 2.20.1
> > > 
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] perf cpumap: Fix snprintf overflow check
@ 2020-03-25 12:19       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-25 12:19 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: Arnaldo Carvalho de Melo, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung, kan.liang, zhe.he, dzickus,
	jstancek, David Laight, linux-kernel, kernel-janitors

Em Tue, Mar 24, 2020 at 03:44:15PM +0100, Christophe JAILLET escreveu:
> Le 24/03/2020 à 13:50, Arnaldo Carvalho de Melo a écrit :
> > Em Tue, Mar 24, 2020 at 08:03:19AM +0100, Christophe JAILLET escreveu:
> > > 'snprintf' returns the number of characters which would be generated for
> > > the given input.
> > > 
> > > If the returned value is *greater than* or equal to the buffer size, it
> > > means that the output has been truncated.
> > > 
> > > Fix the overflow test accordingling.
> >                                    y
> > 
> > You forgot to CC David and add this to your patch, which I did:
> > 
> > Suggested-by: David Laight <David.Laight@ACULAB.COM>
> > 
> > Ok?
> 
> No problem for me.
> 
> The to: and cc: list are (at least should be :)) the ones given by
> get_maintainer, and I was not aware of the 'Suggested-by' tag.

Sure, that is the starting point, but if someone participates in the
discussion and provides suggestions, comments, then it is sensible to
add them to the CC list in follow up patches.
 
> BTW, thanks for fixing the typo in the description.

Part of my job as a maintainer :-)

- Arnaldo
 
> CJ
> 
> 
> > - Arnaldo
> > > Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
> > > Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
> > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > > ---
> > > V2: keep snprintf
> > >      modifiy the tests for truncated output
> > >      Update subject and description
> > > ---
> > >   tools/perf/util/cpumap.c | 10 +++++-----
> > >   1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > > index 983b7388f22b..dc5c5e6fc502 100644
> > > --- a/tools/perf/util/cpumap.c
> > > +++ b/tools/perf/util/cpumap.c
> > > @@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
> > >   	/* get the highest possible cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
> > > -	if (ret = PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
> > >   	/* get the highest present cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
> > > -	if (ret = PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -356,7 +356,7 @@ static void set_max_node_num(void)
> > >   	/* get the highest possible cpu number for a sparse allocation */
> > >   	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
> > > -	if (ret = PATH_MAX) {
> > > +	if (ret >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		goto out;
> > >   	}
> > > @@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
> > >   		return 0;
> > >   	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
> > > -	if (n = PATH_MAX) {
> > > +	if (n >= PATH_MAX) {
> > >   		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   		return -1;
> > >   	}
> > > @@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
> > >   			continue;
> > >   		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
> > > -		if (n = PATH_MAX) {
> > > +		if (n >= PATH_MAX) {
> > >   			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
> > >   			continue;
> > >   		}
> > > -- 
> > > 2.20.1
> > > 
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip: perf/urgent] perf cpumap: Fix snprintf overflow check
  2020-03-24  7:03 ` Christophe JAILLET
@ 2020-04-04  8:41   ` tip-bot2 for Christophe JAILLET
  -1 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Christophe JAILLET @ 2020-04-04  8:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Christophe JAILLET, David Laight, Alexander Shishkin, Don Zickus,
	He Zhe, Jan Stancek, Jiri Olsa, Kan Liang, Mark Rutland,
	Namhyung Kim, Peter Zijlstra, kernel-janitors,
	Arnaldo Carvalho de Melo, x86, LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     d74b181a028bb5a468f0c609553eff6a8fdf4887
Gitweb:        https://git.kernel.org/tip/d74b181a028bb5a468f0c609553eff6a8fdf4887
Author:        Christophe JAILLET <christophe.jaillet@wanadoo.fr>
AuthorDate:    Tue, 24 Mar 2020 08:03:19 +01:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 24 Mar 2020 10:36:00 -03:00

perf cpumap: Fix snprintf overflow check

'snprintf' returns the number of characters which would be generated for
the given input.

If the returned value is *greater than* or equal to the buffer size, it
means that the output has been truncated.

Fix the overflow test accordingly.

Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Suggested-by: David Laight <David.Laight@ACULAB.COM>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: He Zhe <zhe.he@windriver.com>
Cc: Jan Stancek <jstancek@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: kernel-janitors@vger.kernel.org
Link: http://lore.kernel.org/lkml/20200324070319.10901-1-christophe.jaillet@wanadoo.fr
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 983b738..dc5c5e6 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest present cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -356,7 +356,7 @@ static void set_max_node_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
-	if (ret == PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
 		return 0;
 
 	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
-	if (n == PATH_MAX) {
+	if (n >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		return -1;
 	}
@@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
 			continue;
 
 		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
-		if (n == PATH_MAX) {
+		if (n >= PATH_MAX) {
 			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 			continue;
 		}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip: perf/urgent] perf cpumap: Fix snprintf overflow check
@ 2020-04-04  8:41   ` tip-bot2 for Christophe JAILLET
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Christophe JAILLET @ 2020-04-04  8:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Christophe JAILLET, David Laight, Alexander Shishkin, Don Zickus,
	He Zhe, Jan Stancek, Jiri Olsa, Kan Liang, Mark Rutland,
	Namhyung Kim, Peter Zijlstra, kernel-janitors,
	Arnaldo Carvalho de Melo, x86, LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     d74b181a028bb5a468f0c609553eff6a8fdf4887
Gitweb:        https://git.kernel.org/tip/d74b181a028bb5a468f0c609553eff6a8fdf4887
Author:        Christophe JAILLET <christophe.jaillet@wanadoo.fr>
AuthorDate:    Tue, 24 Mar 2020 08:03:19 +01:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 24 Mar 2020 10:36:00 -03:00

perf cpumap: Fix snprintf overflow check

'snprintf' returns the number of characters which would be generated for
the given input.

If the returned value is *greater than* or equal to the buffer size, it
means that the output has been truncated.

Fix the overflow test accordingly.

Fixes: 7780c25bae59f ("perf tools: Allow ability to map cpus to nodes easily")
Fixes: 92a7e1278005b ("perf cpumap: Add cpu__max_present_cpu()")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Suggested-by: David Laight <David.Laight@ACULAB.COM>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: He Zhe <zhe.he@windriver.com>
Cc: Jan Stancek <jstancek@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: kernel-janitors@vger.kernel.org
Link: http://lore.kernel.org/lkml/20200324070319.10901-1-christophe.jaillet@wanadoo.fr
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 983b738..dc5c5e6 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -317,7 +317,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/possible", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -328,7 +328,7 @@ static void set_max_cpu_num(void)
 
 	/* get the highest present cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/cpu/present", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -356,7 +356,7 @@ static void set_max_node_num(void)
 
 	/* get the highest possible cpu number for a sparse allocation */
 	ret = snprintf(path, PATH_MAX, "%s/devices/system/node/possible", mnt);
-	if (ret = PATH_MAX) {
+	if (ret >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		goto out;
 	}
@@ -441,7 +441,7 @@ int cpu__setup_cpunode_map(void)
 		return 0;
 
 	n = snprintf(path, PATH_MAX, "%s/devices/system/node", mnt);
-	if (n = PATH_MAX) {
+	if (n >= PATH_MAX) {
 		pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 		return -1;
 	}
@@ -456,7 +456,7 @@ int cpu__setup_cpunode_map(void)
 			continue;
 
 		n = snprintf(buf, PATH_MAX, "%s/%s", path, dent1->d_name);
-		if (n = PATH_MAX) {
+		if (n >= PATH_MAX) {
 			pr_err("sysfs path crossed PATH_MAX(%d) size\n", PATH_MAX);
 			continue;
 		}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-04-04  8:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24  7:03 [PATCH V2] perf cpumap: Fix snprintf overflow check Christophe JAILLET
2020-03-24  7:03 ` Christophe JAILLET
2020-03-24 12:50 ` Arnaldo Carvalho de Melo
2020-03-24 12:50   ` Arnaldo Carvalho de Melo
2020-03-24 14:44   ` Christophe JAILLET
2020-03-24 14:44     ` Christophe JAILLET
2020-03-25 12:19     ` Arnaldo Carvalho de Melo
2020-03-25 12:19       ` Arnaldo Carvalho de Melo
2020-04-04  8:41 ` [tip: perf/urgent] " tip-bot2 for Christophe JAILLET
2020-04-04  8:41   ` tip-bot2 for Christophe JAILLET

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.