* [PATCH] perf jvmti: Fix gcc string overflow warning
@ 2019-05-31 8:03 Jiri Olsa
2019-05-31 12:05 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2019-05-31 8:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ben Gainey, Stephane Eranian, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra
We are getting fake gcc warning when we compile with gcc9 (9.1.1):
CC jvmti/libjvmti.o
In file included from /usr/include/string.h:494,
from jvmti/libjvmti.c:5:
In function ‘strncpy’,
inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’:
jvmti/libjvmti.c:165:26: note: length computed here
165 | size_t file_name_len = strlen(file_name);
| ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
First I wanted to disable the check, but now I think the code
could be more straight forward. There's no need to check the
source size, strncpy will do that. We just need to make sure
the string is correctly terminated.
Cc: Ben Gainey <ben.gainey@arm.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-sve3b63c550wr907e6ui6gx5@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/jvmti/libjvmti.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
index aea7b1fe85aa..00fa0b7f1ad9 100644
--- a/tools/perf/jvmti/libjvmti.c
+++ b/tools/perf/jvmti/libjvmti.c
@@ -162,8 +162,8 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu
result[i] = '\0';
} else {
/* fallback case */
- size_t file_name_len = strlen(file_name);
- strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length);
+ strncpy(result, file_name, max_length - 1);
+ result[max_length - 1] = 0;
}
}
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] perf jvmti: Fix gcc string overflow warning 2019-05-31 8:03 [PATCH] perf jvmti: Fix gcc string overflow warning Jiri Olsa @ 2019-05-31 12:05 ` Arnaldo Carvalho de Melo 2019-05-31 13:13 ` [PATCHv2] " Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2019-05-31 12:05 UTC (permalink / raw) To: Jiri Olsa Cc: Ben Gainey, Stephane Eranian, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra Em Fri, May 31, 2019 at 10:03:07AM +0200, Jiri Olsa escreveu: > We are getting fake gcc warning when we compile with gcc9 (9.1.1): > > CC jvmti/libjvmti.o > In file included from /usr/include/string.h:494, > from jvmti/libjvmti.c:5: > In function ‘strncpy’, > inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: > jvmti/libjvmti.c:165:26: note: length computed here > 165 | size_t file_name_len = strlen(file_name); > | ^~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > First I wanted to disable the check, but now I think the code > could be more straight forward. There's no need to check the > source size, strncpy will do that. We just need to make sure > the string is correctly terminated. > > Cc: Ben Gainey <ben.gainey@arm.com> > Cc: Stephane Eranian <eranian@google.com> > Link: http://lkml.kernel.org/n/tip-sve3b63c550wr907e6ui6gx5@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/jvmti/libjvmti.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > index aea7b1fe85aa..00fa0b7f1ad9 100644 > --- a/tools/perf/jvmti/libjvmti.c > +++ b/tools/perf/jvmti/libjvmti.c > @@ -162,8 +162,8 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu > result[i] = '\0'; > } else { > /* fallback case */ > - size_t file_name_len = strlen(file_name); > - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); > + strncpy(result, file_name, max_length - 1); > + result[max_length - 1] = 0; The usual idiom here, if we stay with strncpy is: strncpy(result, file_name, max_length - 1)[max_length - 1] = 0; one line instead of two, but there are other possibilities, what I've done int these cases in tools/perf/ is to switch to strlcpy, so just flip that 'n' to a 'l' and it should be enough. For that we just have a copy of the kernel's strlcpy() implementation in lib/string.c, and it has this doc: /** * strlcpy - Copy a C-string into a sized buffer * @dest: Where to copy the string to * @src: Where to copy the string from * @size: size of destination buffer * * Compatible with ``*BSD``: the result is always a valid * NUL-terminated string that fits in the buffer (unless, * of course, the buffer size is zero). It does not pad * out the result like strncpy() does. */ The kernel folks moved beyond that and in lib/string.c we have: /** * strscpy - Copy a C-string into a sized buffer * @dest: Where to copy the string to * @src: Where to copy the string from * @count: Size of destination buffer * * Copy the string, or as much of it as fits, into the dest buffer. The * behavior is undefined if the string buffers overlap. The destination * buffer is always NUL terminated, unless it's zero-sized. * * Preferred to strlcpy() since the API doesn't require reading memory * from the src string beyond the specified "count" bytes, and since * the return value is easier to error-check than strlcpy()'s. * In addition, the implementation is robust to the string changing out * from underneath it, unlike the current strlcpy() implementation. * * Preferred to strncpy() since it always returns a valid string, and * doesn't unnecessarily force the tail of the destination buffer to be * zeroed. If zeroing is desired please use strscpy_pad(). * * Return: The number of characters copied (not including the trailing * %NUL) or -E2BIG if the destination buffer wasn't big enough. */ ssize_t strscpy(char *dest, const char *src, size_t count) I think for these needs flipping that 'n' into a 'l' is good enough. - Arnaldo > } > } > > -- > 2.21.0 -- - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] perf jvmti: Fix gcc string overflow warning 2019-05-31 12:05 ` Arnaldo Carvalho de Melo @ 2019-05-31 13:13 ` Jiri Olsa 2019-06-05 12:53 ` Arnaldo Carvalho de Melo ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Jiri Olsa @ 2019-05-31 13:13 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Ben Gainey, Stephane Eranian, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra On Fri, May 31, 2019 at 09:05:30AM -0300, Arnaldo Carvalho de Melo wrote: SNIP > The kernel folks moved beyond that and in lib/string.c we have: > > /** > * strscpy - Copy a C-string into a sized buffer > * @dest: Where to copy the string to > * @src: Where to copy the string from > * @count: Size of destination buffer > * > * Copy the string, or as much of it as fits, into the dest buffer. The > * behavior is undefined if the string buffers overlap. The destination > * buffer is always NUL terminated, unless it's zero-sized. > * > * Preferred to strlcpy() since the API doesn't require reading memory > * from the src string beyond the specified "count" bytes, and since > * the return value is easier to error-check than strlcpy()'s. > * In addition, the implementation is robust to the string changing out > * from underneath it, unlike the current strlcpy() implementation. > * > * Preferred to strncpy() since it always returns a valid string, and > * doesn't unnecessarily force the tail of the destination buffer to be > * zeroed. If zeroing is desired please use strscpy_pad(). > * > * Return: The number of characters copied (not including the trailing > * %NUL) or -E2BIG if the destination buffer wasn't big enough. > */ > ssize_t strscpy(char *dest, const char *src, size_t count) > > > > I think for these needs flipping that 'n' into a 'l' is good enough. ok, I forgot there's strlcpy.. v2 attached thanks, jirka --- We are getting fake gcc warning when we compile with gcc9 (9.1.1): CC jvmti/libjvmti.o In file included from /usr/include/string.h:494, from jvmti/libjvmti.c:5: In function ‘strncpy’, inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: jvmti/libjvmti.c:165:26: note: length computed here 165 | size_t file_name_len = strlen(file_name); | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors As per Arnaldo's suggestion using strlcpy, which does the same thing and keeps gcc silent. Cc: Ben Gainey <ben.gainey@arm.com> Cc: Stephane Eranian <eranian@google.com> Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Link: http://lkml.kernel.org/n/tip-sve3b63c550wr907e6ui6gx5@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/jvmti/libjvmti.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index aea7b1fe85aa..c441a34cb1c0 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/compiler.h> +#include <linux/string.h> #include <sys/types.h> #include <stdio.h> #include <string.h> @@ -162,8 +163,7 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu result[i] = '\0'; } else { /* fallback case */ - size_t file_name_len = strlen(file_name); - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); + strlcpy(result, file_name, max_length); } } -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2] perf jvmti: Fix gcc string overflow warning 2019-05-31 13:13 ` [PATCHv2] " Jiri Olsa @ 2019-06-05 12:53 ` Arnaldo Carvalho de Melo 2019-06-17 19:15 ` [tip:perf/core] perf jvmti: Address gcc string overflow warning for strncpy() tip-bot for Jiri Olsa 2019-07-09 11:33 ` tip-bot for Jiri Olsa 2 siblings, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2019-06-05 12:53 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Ben Gainey, Stephane Eranian, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra Em Fri, May 31, 2019 at 03:13:21PM +0200, Jiri Olsa escreveu: > On Fri, May 31, 2019 at 09:05:30AM -0300, Arnaldo Carvalho de Melo wrote: > > I think for these needs flipping that 'n' into a 'l' is good enough. > ok, I forgot there's strlcpy.. v2 attached Thanks, applied v2. - Arnaldo > --- > We are getting fake gcc warning when we compile with gcc9 (9.1.1): > > CC jvmti/libjvmti.o > In file included from /usr/include/string.h:494, > from jvmti/libjvmti.c:5: > In function ‘strncpy’, > inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: > jvmti/libjvmti.c:165:26: note: length computed here > 165 | size_t file_name_len = strlen(file_name); > | ^~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > As per Arnaldo's suggestion using strlcpy, which does > the same thing and keeps gcc silent. > > Cc: Ben Gainey <ben.gainey@arm.com> > Cc: Stephane Eranian <eranian@google.com> > Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com> > Link: http://lkml.kernel.org/n/tip-sve3b63c550wr907e6ui6gx5@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/jvmti/libjvmti.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > index aea7b1fe85aa..c441a34cb1c0 100644 > --- a/tools/perf/jvmti/libjvmti.c > +++ b/tools/perf/jvmti/libjvmti.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/compiler.h> > +#include <linux/string.h> > #include <sys/types.h> > #include <stdio.h> > #include <string.h> > @@ -162,8 +163,7 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu > result[i] = '\0'; > } else { > /* fallback case */ > - size_t file_name_len = strlen(file_name); > - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); > + strlcpy(result, file_name, max_length); > } > } > > -- > 2.21.0 -- - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] perf jvmti: Address gcc string overflow warning for strncpy() 2019-05-31 13:13 ` [PATCHv2] " Jiri Olsa 2019-06-05 12:53 ` Arnaldo Carvalho de Melo @ 2019-06-17 19:15 ` tip-bot for Jiri Olsa 2019-07-09 11:33 ` tip-bot for Jiri Olsa 2 siblings, 0 replies; 6+ messages in thread From: tip-bot for Jiri Olsa @ 2019-06-17 19:15 UTC (permalink / raw) To: linux-tip-commits Cc: jolsa, acme, mingo, eranian, hpa, ben.gainey, namhyung, tglx, jolsa, linux-kernel, peterz, alexander.shishkin Commit-ID: 279ab04dbea1370d2eac0f854270369ccaef8a44 Gitweb: https://git.kernel.org/tip/279ab04dbea1370d2eac0f854270369ccaef8a44 Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Fri, 31 May 2019 15:13:21 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Wed, 5 Jun 2019 09:51:26 -0300 perf jvmti: Address gcc string overflow warning for strncpy() We are getting false positive gcc warning when we compile with gcc9 (9.1.1): CC jvmti/libjvmti.o In file included from /usr/include/string.h:494, from jvmti/libjvmti.c:5: In function ‘strncpy’, inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: jvmti/libjvmti.c:165:26: note: length computed here 165 | size_t file_name_len = strlen(file_name); | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors As per Arnaldo's suggestion use strlcpy(), which does the same thing and keeps gcc silent. Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ben Gainey <ben.gainey@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/r/20190531131321.GB1281@krava Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/jvmti/libjvmti.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index aea7b1fe85aa..c441a34cb1c0 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/compiler.h> +#include <linux/string.h> #include <sys/types.h> #include <stdio.h> #include <string.h> @@ -162,8 +163,7 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu result[i] = '\0'; } else { /* fallback case */ - size_t file_name_len = strlen(file_name); - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); + strlcpy(result, file_name, max_length); } } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:perf/core] perf jvmti: Address gcc string overflow warning for strncpy() 2019-05-31 13:13 ` [PATCHv2] " Jiri Olsa 2019-06-05 12:53 ` Arnaldo Carvalho de Melo 2019-06-17 19:15 ` [tip:perf/core] perf jvmti: Address gcc string overflow warning for strncpy() tip-bot for Jiri Olsa @ 2019-07-09 11:33 ` tip-bot for Jiri Olsa 2 siblings, 0 replies; 6+ messages in thread From: tip-bot for Jiri Olsa @ 2019-07-09 11:33 UTC (permalink / raw) To: linux-tip-commits Cc: ben.gainey, peterz, alexander.shishkin, hpa, acme, jolsa, mingo, namhyung, tglx, jolsa, eranian, linux-kernel Commit-ID: dab0f4ebb22ee6d16051011d624cff79a99baa8a Gitweb: https://git.kernel.org/tip/dab0f4ebb22ee6d16051011d624cff79a99baa8a Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Fri, 31 May 2019 15:13:21 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Sun, 7 Jul 2019 12:33:32 -0300 perf jvmti: Address gcc string overflow warning for strncpy() We are getting false positive gcc warning when we compile with gcc9 (9.1.1): CC jvmti/libjvmti.o In file included from /usr/include/string.h:494, from jvmti/libjvmti.c:5: In function ‘strncpy’, inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: jvmti/libjvmti.c:165:26: note: length computed here 165 | size_t file_name_len = strlen(file_name); | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors As per Arnaldo's suggestion use strlcpy(), which does the same thing and keeps gcc silent. Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ben Gainey <ben.gainey@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/r/20190531131321.GB1281@krava Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/jvmti/libjvmti.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index aea7b1fe85aa..c441a34cb1c0 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/compiler.h> +#include <linux/string.h> #include <sys/types.h> #include <stdio.h> #include <string.h> @@ -162,8 +163,7 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu result[i] = '\0'; } else { /* fallback case */ - size_t file_name_len = strlen(file_name); - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); + strlcpy(result, file_name, max_length); } } ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-09 11:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-31 8:03 [PATCH] perf jvmti: Fix gcc string overflow warning Jiri Olsa 2019-05-31 12:05 ` Arnaldo Carvalho de Melo 2019-05-31 13:13 ` [PATCHv2] " Jiri Olsa 2019-06-05 12:53 ` Arnaldo Carvalho de Melo 2019-06-17 19:15 ` [tip:perf/core] perf jvmti: Address gcc string overflow warning for strncpy() tip-bot for Jiri Olsa 2019-07-09 11:33 ` tip-bot for Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).