| Message ID | 20190117124014.18308-2-aastier@freebox.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix GCC 8 warnings | expand |
On Thu, Jan 17, 2019 at 01:40:12PM +0100, Anisse Astier wrote: > GCC 8.2 gives this warning: > > builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’: > builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=] > snprintf(dst, len, "/host%s", resolved_path); > ^~ ~~~~~~~~~~~~~ > > It's because it understands that len is PATH_MAX, the same as > resolved_path's size. This patch handles the case where the string is > truncated, and fixes the warning. > > Signed-off-by: Anisse Astier <aastier@freebox.fr> > --- > builtin-run.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/builtin-run.c b/builtin-run.c > index 443c10b..32d8aef 100644 > --- a/builtin-run.c > +++ b/builtin-run.c > @@ -414,7 +414,9 @@ static void resolve_program(const char *src, char *dst, size_t len) > if (!realpath(src, resolved_path)) > die("Unable to resolve program %s: %s\n", src, strerror(errno)); > > - snprintf(dst, len, "/host%s", resolved_path); > + if (snprintf(dst, len, "/host%s", resolved_path) >= (int)len - 1) > + die("Pathname too long: %s -> %s\n", src, resolved_path); Why do we need the '- 1' here? Will
Hi Will, Thanks for taking the time to review this patch, On Fri, Feb 01, 2019 at 04:27:16PM +0000, Will Deacon wrote: > On Thu, Jan 17, 2019 at 01:40:12PM +0100, Anisse Astier wrote: > > GCC 8.2 gives this warning: > > > > builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’: > > builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=] > > snprintf(dst, len, "/host%s", resolved_path); > > ^~ ~~~~~~~~~~~~~ > > > > It's because it understands that len is PATH_MAX, the same as > > resolved_path's size. This patch handles the case where the string is > > truncated, and fixes the warning. > > > > Signed-off-by: Anisse Astier <aastier@freebox.fr> > > --- > > builtin-run.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/builtin-run.c b/builtin-run.c > > index 443c10b..32d8aef 100644 > > --- a/builtin-run.c > > +++ b/builtin-run.c > > @@ -414,7 +414,9 @@ static void resolve_program(const char *src, char *dst, size_t len) > > if (!realpath(src, resolved_path)) > > die("Unable to resolve program %s: %s\n", src, strerror(errno)); > > > > - snprintf(dst, len, "/host%s", resolved_path); > > + if (snprintf(dst, len, "/host%s", resolved_path) >= (int)len - 1) > > + die("Pathname too long: %s -> %s\n", src, resolved_path); > > Why do we need the '- 1' here? > > Will Good catch, I think it's not needed in this case. I'll send an updated patch. Anisse
diff --git a/builtin-run.c b/builtin-run.c index 443c10b..32d8aef 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -414,7 +414,9 @@ static void resolve_program(const char *src, char *dst, size_t len) if (!realpath(src, resolved_path)) die("Unable to resolve program %s: %s\n", src, strerror(errno)); - snprintf(dst, len, "/host%s", resolved_path); + if (snprintf(dst, len, "/host%s", resolved_path) >= (int)len - 1) + die("Pathname too long: %s -> %s\n", src, resolved_path); + } else strncpy(dst, src, len); }
GCC 8.2 gives this warning: builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’: builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=] snprintf(dst, len, "/host%s", resolved_path); ^~ ~~~~~~~~~~~~~ It's because it understands that len is PATH_MAX, the same as resolved_path's size. This patch handles the case where the string is truncated, and fixes the warning. Signed-off-by: Anisse Astier <aastier@freebox.fr> --- builtin-run.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)