[PATCH] tools lib api fs: fix gcc9 compilation error

[PATCH] tools lib api fs: fix gcc9 compilation error

[Andrey Zhizhikin]

GCC9 introduced string hardening mechanisms, which exhibits the error
during fs api compilation:

error: '__builtin_strncpy' specified bound 4096 equals destination size
[-Werror=stringop-truncation]

This comes when the length of copy passed to strncpy is is equal to
destination size, which could potentially lead to buffer overflow.

There is a need to mitigate this potential issue by limiting the size of
destination by 1 and explicitly terminate the destination with NULL.

Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/api/fs/fs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 11b3885e833e..027b18f7ed8c 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
 	size_t name_len = strlen(fs->name);
 	/* name + "_PATH" + '\0' */
 	char upper_name[name_len + 5 + 1];
+
 	memcpy(upper_name, fs->name, name_len);
 	mem_toupper(upper_name, name_len);
 	strcpy(&upper_name[name_len], "_PATH");
@@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
 		return false;
 
 	fs->found = true;
-	strncpy(fs->path, override_path, sizeof(fs->path));
+	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
+	fs->path[sizeof(fs->path) - 1] = '\0';
 	return true;
 }
 
-- 
2.17.1

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Andrey Zhizhikin]

Hello all,

I'd like to have a gentle ping on this patch, if someone could review
and apply it - I'd really appreciate it.

On Wed, Dec 11, 2019 at 9:01 AM Andrey Zhizhikin <andrey.z@gmail.com> wrote:
>
> GCC9 introduced string hardening mechanisms, which exhibits the error
> during fs api compilation:
>
> error: '__builtin_strncpy' specified bound 4096 equals destination size
> [-Werror=stringop-truncation]
>
> This comes when the length of copy passed to strncpy is is equal to
> destination size, which could potentially lead to buffer overflow.
>
> There is a need to mitigate this potential issue by limiting the size of
> destination by 1 and explicitly terminate the destination with NULL.
>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/api/fs/fs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index 11b3885e833e..027b18f7ed8c 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
>         size_t name_len = strlen(fs->name);
>         /* name + "_PATH" + '\0' */
>         char upper_name[name_len + 5 + 1];
> +
>         memcpy(upper_name, fs->name, name_len);
>         mem_toupper(upper_name, name_len);
>         strcpy(&upper_name[name_len], "_PATH");
> @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
>                 return false;
>
>         fs->found = true;
> -       strncpy(fs->path, override_path, sizeof(fs->path));
> +       strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> +       fs->path[sizeof(fs->path) - 1] = '\0';
>         return true;
>  }
>
> --
> 2.17.1
>


-- 
Regards,
Andrey.

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Sergey Senozhatsky]

On (19/12/11 08:01), Andrey Zhizhikin wrote:
[..]
> @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
>  	size_t name_len = strlen(fs->name);
>  	/* name + "_PATH" + '\0' */
>  	char upper_name[name_len + 5 + 1];
> +
>  	memcpy(upper_name, fs->name, name_len);
>  	mem_toupper(upper_name, name_len);
>  	strcpy(&upper_name[name_len], "_PATH");
> @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
>  		return false;
>  
>  	fs->found = true;
> -	strncpy(fs->path, override_path, sizeof(fs->path));
> +	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> +	fs->path[sizeof(fs->path) - 1] = '\0';

I think the trend these days is to prefer stracpy/strscpy over
strcpy/strlcpy/strncpy.

	-ss

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Sergey Senozhatsky]

On (19/12/18 12:10), Sergey Senozhatsky wrote:
> On (19/12/11 08:01), Andrey Zhizhikin wrote:
> [..]
> > @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
> >  	size_t name_len = strlen(fs->name);
> >  	/* name + "_PATH" + '\0' */
> >  	char upper_name[name_len + 5 + 1];
> > +
> >  	memcpy(upper_name, fs->name, name_len);
> >  	mem_toupper(upper_name, name_len);
> >  	strcpy(&upper_name[name_len], "_PATH");
> > @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
> >  		return false;
> >  
> >  	fs->found = true;
> > -	strncpy(fs->path, override_path, sizeof(fs->path));
> > +	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> > +	fs->path[sizeof(fs->path) - 1] = '\0';
> 
> I think the trend these days is to prefer stracpy/strscpy over
> strcpy/strlcpy/strncpy.

Scratch that. This is user-space, I should pay more attention.

	-ss

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Jiri Olsa]

On Wed, Dec 11, 2019 at 08:01:09AM +0000, Andrey Zhizhikin wrote:
> GCC9 introduced string hardening mechanisms, which exhibits the error
> during fs api compilation:
> 
> error: '__builtin_strncpy' specified bound 4096 equals destination size
> [-Werror=stringop-truncation]
> 
> This comes when the length of copy passed to strncpy is is equal to
> destination size, which could potentially lead to buffer overflow.
> 
> There is a need to mitigate this potential issue by limiting the size of
> destination by 1 and explicitly terminate the destination with NULL.
> 
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/lib/api/fs/fs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index 11b3885e833e..027b18f7ed8c 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
>  	size_t name_len = strlen(fs->name);
>  	/* name + "_PATH" + '\0' */
>  	char upper_name[name_len + 5 + 1];
> +
>  	memcpy(upper_name, fs->name, name_len);
>  	mem_toupper(upper_name, name_len);
>  	strcpy(&upper_name[name_len], "_PATH");
> @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
>  		return false;
>  
>  	fs->found = true;
> -	strncpy(fs->path, override_path, sizeof(fs->path));
> +	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> +	fs->path[sizeof(fs->path) - 1] = '\0';
>  	return true;
>  }
>  
> -- 
> 2.17.1
> 

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Petr Mladek]

On Wed 2019-12-11 08:01:09, Andrey Zhizhikin wrote:
> GCC9 introduced string hardening mechanisms, which exhibits the error
> during fs api compilation:
> 
> error: '__builtin_strncpy' specified bound 4096 equals destination size
> [-Werror=stringop-truncation]
> 
> This comes when the length of copy passed to strncpy is is equal to
> destination size, which could potentially lead to buffer overflow.
> 
> There is a need to mitigate this potential issue by limiting the size of
> destination by 1 and explicitly terminate the destination with NULL.
> 
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Arnaldo Carvalho de Melo]

Em Wed, Dec 11, 2019 at 08:01:09AM +0000, Andrey Zhizhikin escreveu:
> GCC9 introduced string hardening mechanisms, which exhibits the error
> during fs api compilation:
> 
> error: '__builtin_strncpy' specified bound 4096 equals destination size
> [-Werror=stringop-truncation]
> 
> This comes when the length of copy passed to strncpy is is equal to
> destination size, which could potentially lead to buffer overflow.
> 
> There is a need to mitigate this potential issue by limiting the size of
> destination by 1 and explicitly terminate the destination with NULL.

Thanks, applied and collected the reviewed-by and acked-by provided,

- Arnaldo
 
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/api/fs/fs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index 11b3885e833e..027b18f7ed8c 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
>  	size_t name_len = strlen(fs->name);
>  	/* name + "_PATH" + '\0' */
>  	char upper_name[name_len + 5 + 1];
> +
>  	memcpy(upper_name, fs->name, name_len);
>  	mem_toupper(upper_name, name_len);
>  	strcpy(&upper_name[name_len], "_PATH");
> @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
>  		return false;
>  
>  	fs->found = true;
> -	strncpy(fs->path, override_path, sizeof(fs->path));
> +	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> +	fs->path[sizeof(fs->path) - 1] = '\0';
>  	return true;
>  }
>  
> -- 
> 2.17.1

-- 

- Arnaldo

Re: [PATCH] tools lib api fs: fix gcc9 compilation error

[Andrey Zhizhikin]

On Wed, Dec 18, 2019 at 3:33 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Wed, Dec 11, 2019 at 08:01:09AM +0000, Andrey Zhizhikin escreveu:
> > GCC9 introduced string hardening mechanisms, which exhibits the error
> > during fs api compilation:
> >
> > error: '__builtin_strncpy' specified bound 4096 equals destination size
> > [-Werror=stringop-truncation]
> >
> > This comes when the length of copy passed to strncpy is is equal to
> > destination size, which could potentially lead to buffer overflow.
> >
> > There is a need to mitigate this potential issue by limiting the size of
> > destination by 1 and explicitly terminate the destination with NULL.
>
> Thanks, applied and collected the reviewed-by and acked-by provided,

Thanks a lot for review and collecting this patch!

>
> - Arnaldo
>
> > Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/api/fs/fs.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> > index 11b3885e833e..027b18f7ed8c 100644
> > --- a/tools/lib/api/fs/fs.c
> > +++ b/tools/lib/api/fs/fs.c
> > @@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
> >       size_t name_len = strlen(fs->name);
> >       /* name + "_PATH" + '\0' */
> >       char upper_name[name_len + 5 + 1];
> > +
> >       memcpy(upper_name, fs->name, name_len);
> >       mem_toupper(upper_name, name_len);
> >       strcpy(&upper_name[name_len], "_PATH");
> > @@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
> >               return false;
> >
> >       fs->found = true;
> > -     strncpy(fs->path, override_path, sizeof(fs->path));
> > +     strncpy(fs->path, override_path, sizeof(fs->path) - 1);
> > +     fs->path[sizeof(fs->path) - 1] = '\0';
> >       return true;
> >  }
> >
> > --
> > 2.17.1
>
> --
>
> - Arnaldo

-- 
Regards,
Andrey.

[tip: perf/core] tools lib api fs: Fix gcc9 stringop-truncation compilation error

[tip-bot2 for Andrey Zhizhikin]

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

Commit-ID:     6794200fa3c9c3e6759dae099145f23e4310f4f7
Gitweb:        https://git.kernel.org/tip/6794200fa3c9c3e6759dae099145f23e4310f4f7
Author:        Andrey Zhizhikin <andrey.z@gmail.com>
AuthorDate:    Wed, 11 Dec 2019 08:01:09 
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 06 Jan 2020 11:46:09 -03:00

tools lib api fs: Fix gcc9 stringop-truncation compilation error

GCC9 introduced string hardening mechanisms, which exhibits the error
during fs api compilation:

error: '__builtin_strncpy' specified bound 4096 equals destination size
[-Werror=stringop-truncation]

This comes when the length of copy passed to strncpy is is equal to
destination size, which could potentially lead to buffer overflow.

There is a need to mitigate this potential issue by limiting the size of
destination by 1 and explicitly terminate the destination with NULL.

Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
Link: http://lore.kernel.org/lkml/20191211080109.18765-1-andrey.zhizhikin@leica-geosystems.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 11b3885..027b18f 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -210,6 +210,7 @@ static bool fs__env_override(struct fs *fs)
 	size_t name_len = strlen(fs->name);
 	/* name + "_PATH" + '\0' */
 	char upper_name[name_len + 5 + 1];
+
 	memcpy(upper_name, fs->name, name_len);
 	mem_toupper(upper_name, name_len);
 	strcpy(&upper_name[name_len], "_PATH");
@@ -219,7 +220,8 @@ static bool fs__env_override(struct fs *fs)
 		return false;
 
 	fs->found = true;
-	strncpy(fs->path, override_path, sizeof(fs->path));
+	strncpy(fs->path, override_path, sizeof(fs->path) - 1);
+	fs->path[sizeof(fs->path) - 1] = '\0';
 	return true;
 }