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");
-- 
2.20.1

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");