| Message ID | 20190321170831.6539-2-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | trace: fix compilation issues for QEMU 4.0.0 | expand |
Stefan Hajnoczi <stefanha@redhat.com> writes: > If the tracefs mountpoint has a very long path we may exceed PATH_MAX. > This is a system misconfiguration and the user must resolve it so that > applications can perform path-based system calls successfully. > > This issue does not occur on real-world systems since tracefs is mounted > on /sys/kernel/debug/tracing/, but the compiler is smart enough to > foresee the possibility and warn about the unchecked snprintf(3) return > value. This patch fixes the compiler warning. > > Reported-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > trace/ftrace.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/trace/ftrace.c b/trace/ftrace.c > index 61692a8682..9749543d9b 100644 > --- a/trace/ftrace.c > +++ b/trace/ftrace.c > @@ -53,7 +53,11 @@ bool ftrace_init(void) > } > > if (tracefs_found) { > - snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir); > + if (snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir) > + >= sizeof(path)) { > + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); > + return false; > + } > trace_fd = open(path, O_WRONLY); > if (trace_fd < 0) { > if (errno == EACCES) { > @@ -72,7 +76,11 @@ bool ftrace_init(void) > } > close(trace_fd); > } > - snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir); > + if (snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir) > + >= sizeof(path)) { > + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); > + return false; > + } > trace_marker_fd = open(path, O_WRONLY); > if (trace_marker_fd < 0) { > perror("Could not open ftrace 'trace_marker' file"); If we expected these errors to happen, then I'd suggest to include the problematic mount point in the error message. Since we don't, it doesn't feel worth the bother. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 21/03/2019 17:08, Stefan Hajnoczi wrote: > If the tracefs mountpoint has a very long path we may exceed PATH_MAX. > This is a system misconfiguration and the user must resolve it so that > applications can perform path-based system calls successfully. > > This issue does not occur on real-world systems since tracefs is mounted > on /sys/kernel/debug/tracing/, but the compiler is smart enough to > foresee the possibility and warn about the unchecked snprintf(3) return > value. This patch fixes the compiler warning. > > Reported-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Liam Merwick <liam.merwick@oracle.com> > --- > trace/ftrace.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/trace/ftrace.c b/trace/ftrace.c > index 61692a8682..9749543d9b 100644 > --- a/trace/ftrace.c > +++ b/trace/ftrace.c > @@ -53,7 +53,11 @@ bool ftrace_init(void) > } > > if (tracefs_found) { > - snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir); > + if (snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir) > + >= sizeof(path)) { > + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); > + return false; > + } > trace_fd = open(path, O_WRONLY); > if (trace_fd < 0) { > if (errno == EACCES) { > @@ -72,7 +76,11 @@ bool ftrace_init(void) > } > close(trace_fd); > } > - snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir); > + if (snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir) > + >= sizeof(path)) { > + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); > + return false; > + } > trace_marker_fd = open(path, O_WRONLY); > if (trace_marker_fd < 0) { > perror("Could not open ftrace 'trace_marker' file");
diff --git a/trace/ftrace.c b/trace/ftrace.c index 61692a8682..9749543d9b 100644 --- a/trace/ftrace.c +++ b/trace/ftrace.c @@ -53,7 +53,11 @@ bool ftrace_init(void) } if (tracefs_found) { - snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir); + if (snprintf(path, PATH_MAX, "%s%s/tracing_on", mount_point, subdir) + >= sizeof(path)) { + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); + return false; + } trace_fd = open(path, O_WRONLY); if (trace_fd < 0) { if (errno == EACCES) { @@ -72,7 +76,11 @@ bool ftrace_init(void) } close(trace_fd); } - snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir); + if (snprintf(path, PATH_MAX, "%s%s/trace_marker", mount_point, subdir) + >= sizeof(path)) { + fprintf(stderr, "Using tracefs mountpoint would exceed PATH_MAX\n"); + return false; + } trace_marker_fd = open(path, O_WRONLY); if (trace_marker_fd < 0) { perror("Could not open ftrace 'trace_marker' file");
If the tracefs mountpoint has a very long path we may exceed PATH_MAX. This is a system misconfiguration and the user must resolve it so that applications can perform path-based system calls successfully. This issue does not occur on real-world systems since tracefs is mounted on /sys/kernel/debug/tracing/, but the compiler is smart enough to foresee the possibility and warn about the unchecked snprintf(3) return value. This patch fixes the compiler warning. Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- trace/ftrace.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)