[PATCH 12/18] test_power: change strncpy+truncation to strlcpy

[PATCH 12/18] test_power: change strncpy+truncation to strlcpy

[Dominique Martinet]

Generated by scripts/coccinelle/misc/strncpy_truncation.cocci

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---

Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch

 drivers/power/supply/test_power.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
index 57246cdbd042..64adf630f64f 100644
--- a/drivers/power/supply/test_power.c
+++ b/drivers/power/supply/test_power.c
@@ -297,8 +297,7 @@ static int map_get_value(struct battery_property_map *map, const char *key,
 	char buf[MAX_KEYLENGTH];
 	int cr;
 
-	strncpy(buf, key, MAX_KEYLENGTH);
-	buf[MAX_KEYLENGTH-1] = '\0';
+	strlcpy(buf, key, MAX_KEYLENGTH);
 
 	cr = strnlen(buf, MAX_KEYLENGTH) - 1;
 	if (cr < 0)
-- 
2.17.1

[PATCH 18/18] cpupower: change strncpy+truncation to strlcpy

[Dominique Martinet]

Generated by scripts/coccinelle/misc/strncpy_truncation.cocci

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---

Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch

 tools/power/cpupower/bench/parse.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c
index 9ba8a44ad2a7..1566b89989b2 100644
--- a/tools/power/cpupower/bench/parse.c
+++ b/tools/power/cpupower/bench/parse.c
@@ -221,9 +221,8 @@ int prepare_config(const char *path, struct config *config)
 			sscanf(val, "%u", &config->cpu);
 
 		else if (strcmp("governor", opt) == 0) {
-			strncpy(config->governor, val,
+			strlcpy(config->governor, val,
 					sizeof(config->governor));
-			config->governor[sizeof(config->governor) - 1] = '\0';
 		}
 
 		else if (strcmp("priority", opt) == 0) {
-- 
2.17.1

Re: [PATCH 18/18] cpupower: change strncpy+truncation to strlcpy

[Shuah Khan]

On 07/12/2018 07:26 PM, Dominique Martinet wrote:
> Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
> 
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
> 
> Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
> first patch of the serie) for the motivation behind this patch
> 
>  tools/power/cpupower/bench/parse.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c
> index 9ba8a44ad2a7..1566b89989b2 100644
> --- a/tools/power/cpupower/bench/parse.c
> +++ b/tools/power/cpupower/bench/parse.c
> @@ -221,9 +221,8 @@ int prepare_config(const char *path, struct config *config)
>  			sscanf(val, "%u", &config->cpu);
>  
>  		else if (strcmp("governor", opt) == 0) {
> -			strncpy(config->governor, val,
> +			strlcpy(config->governor, val,
>  					sizeof(config->governor));
> -			config->governor[sizeof(config->governor) - 1] = '\0';
>  		}
>  
>  		else if (strcmp("priority", opt) == 0) {
> 

Thanks for the patch. I will pull this in for 4.19-rc1

-- Shuah

Re: [PATCH 18/18] cpupower: change strncpy+truncation to strlcpy

[Daniel Díaz]

Hello!


On 12 July 2018 at 20:26, Dominique Martinet <asmadeus@codewreck.org> wrote:
> Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
>
> Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
> first patch of the serie) for the motivation behind this patch
>
>  tools/power/cpupower/bench/parse.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c
> index 9ba8a44ad2a7..1566b89989b2 100644
> --- a/tools/power/cpupower/bench/parse.c
> +++ b/tools/power/cpupower/bench/parse.c
> @@ -221,9 +221,8 @@ int prepare_config(const char *path, struct config *config)
>                         sscanf(val, "%u", &config->cpu);
>
>                 else if (strcmp("governor", opt) == 0) {
> -                       strncpy(config->governor, val,
> +                       strlcpy(config->governor, val,
>                                         sizeof(config->governor));
> -                       config->governor[sizeof(config->governor) - 1] = '\0';
>                 }
>
>                 else if (strcmp("priority", opt) == 0) {
> --
> 2.17.1

I can't get cpupower to compile anymore now that it made its way to linux-next:
  [/linux/tools/power/cpupower]$ make
    CC       lib/cpufreq.o
  [...]
  make[1]: Entering directory '/linux/tools/power/cpupower/bench'
    CC       main.o
    CC       parse.o
  parse.c: In function ‘prepare_config’:
  parse.c:224:4: warning: implicit declaration of function ‘strlcpy’
[-Wimplicit-function-declaration]
      strlcpy(config->governor, val,
      ^
    CC       system.o
    CC       benchmark.o
    CC       cpufreq-bench
  .//parse.o: In function `prepare_config':
  /linux/tools/power/cpupower/bench/parse.c:224: undefined reference
to `strlcpy'
  collect2: error: ld returned 1 exit status
  Makefile:25: recipe for target 'cpufreq-bench' failed
  make[1]: *** [cpufreq-bench] Error 1
  make[1]: Leaving directory '/linux/tools/power/cpupower/bench'
  Makefile:258: recipe for target 'compile-bench' failed
  make: *** [compile-bench] Error 2

Does it need anything special to make?

Greetings!

Daniel Díaz
daniel.diaz@linaro.org

Re: [PATCH 18/18] cpupower: change strncpy+truncation to strlcpy

[Dominique Martinet]

Daniel Díaz wrote on Tue, Aug 14, 2018:
> I can't get cpupower to compile anymore now that it made its way to linux-next:
>   [/linux/tools/power/cpupower]$ make
>     CC       lib/cpufreq.o
>   [...]
>   make[1]: Entering directory '/linux/tools/power/cpupower/bench'
>     CC       main.o
>     CC       parse.o
>   parse.c: In function ‘prepare_config’:
>   parse.c:224:4: warning: implicit declaration of function ‘strlcpy’
> [-Wimplicit-function-declaration]
>       strlcpy(config->governor, val,
>       ^
>     CC       system.o
>     CC       benchmark.o
>     CC       cpufreq-bench
>   .//parse.o: In function `prepare_config':
>   /linux/tools/power/cpupower/bench/parse.c:224: undefined reference
> to `strlcpy'
>   collect2: error: ld returned 1 exit status
>   Makefile:25: recipe for target 'cpufreq-bench' failed
>   make[1]: *** [cpufreq-bench] Error 1
>   make[1]: Leaving directory '/linux/tools/power/cpupower/bench'
>   Makefile:258: recipe for target 'compile-bench' failed
>   make: *** [compile-bench] Error 2
> 
> Does it need anything special to make?

Ugh, no, I am really ashamed about this patch series for insufficient
testing in general. It is currently "under rework" for an indefinite
time frame as I have had other priorities but I'll add cpupower to the
list...
More precisely, the function is defined in the linux kernel but for
userspace strlcpy is only available through libbsd, and I don't believe
we should pull that in just for this.

I'll send a second patch using snprintf and warning if a truncation
occurs (which is the proper fix that the gcc folks intended people to do
anyway) when I get around to it, but I would recommend to just revert
the patch for now.


Shuah, could you take the patch off please if you haven't pushed it to
linus yet?


Sorry for the time you might have spent on this,
-- 
Dominique Martinet

Re: [PATCH 18/18] cpupower: change strncpy+truncation to strlcpy

[Shuah Khan]

On 08/14/2018 01:27 PM, Dominique Martinet wrote:
> Daniel Díaz wrote on Tue, Aug 14, 2018:
>> I can't get cpupower to compile anymore now that it made its way to linux-next:
>>   [/linux/tools/power/cpupower]$ make
>>     CC       lib/cpufreq.o
>>   [...]
>>   make[1]: Entering directory '/linux/tools/power/cpupower/bench'
>>     CC       main.o
>>     CC       parse.o
>>   parse.c: In function ‘prepare_config’:
>>   parse.c:224:4: warning: implicit declaration of function ‘strlcpy’
>> [-Wimplicit-function-declaration]
>>       strlcpy(config->governor, val,
>>       ^
>>     CC       system.o
>>     CC       benchmark.o
>>     CC       cpufreq-bench
>>   .//parse.o: In function `prepare_config':
>>   /linux/tools/power/cpupower/bench/parse.c:224: undefined reference
>> to `strlcpy'
>>   collect2: error: ld returned 1 exit status
>>   Makefile:25: recipe for target 'cpufreq-bench' failed
>>   make[1]: *** [cpufreq-bench] Error 1
>>   make[1]: Leaving directory '/linux/tools/power/cpupower/bench'
>>   Makefile:258: recipe for target 'compile-bench' failed
>>   make: *** [compile-bench] Error 2
>>
>> Does it need anything special to make?
> 
> Ugh, no, I am really ashamed about this patch series for insufficient
> testing in general. It is currently "under rework" for an indefinite
> time frame as I have had other priorities but I'll add cpupower to the
> list...
> More precisely, the function is defined in the linux kernel but for
> userspace strlcpy is only available through libbsd, and I don't believe
> we should pull that in just for this.
> 
> I'll send a second patch using snprintf and warning if a truncation
> occurs (which is the proper fix that the gcc folks intended people to do
> anyway) when I get around to it, but I would recommend to just revert
> the patch for now.
> 
> 
> Shuah, could you take the patch off please if you haven't pushed it to
> linus yet?
> 
> 
> Sorry for the time you might have spent on this,
> 
I will go ahead and revert it.

thanks,
-- Shuah