| Message ID | 1531445169-19912-1-git-send-email-asmadeus@codewreck.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Shuah Khan |
| Headers | show |
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
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
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,
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
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) {
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(-)