[PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[Kees Cook]

Replace the deprecated[1] strncpy() with strscpy_pad() in the
xfile_create and xmbuf_create tracepoints.

Both tracepoints use file_path() to resolve a pathname into
__entry->pathname (a char[MAXNAMELEN] trace ring buffer field). On
failure, the error path overwrites the buffer with the string literal
"(unknown)" via strncpy(). The original strncpy() zero-pads the
remaining 246 bytes (MAXNAMELEN is 256, "(unknown)" is 10 bytes
including NUL).

strscpy_pad() preserves this zero-padding, which matters because the
destination is a trace ring buffer entry: ring buffer slots are not
zeroed on allocation, and the raw buffer is readable by userspace via
tracefs. The zero-padding ensures no stale data remains in the
buffer after the error path overwrites it.

The source is a 10-byte string literal into a 256-byte destination,
so there is no behavioral change.

Link: https://github.com/KSPP/linux/issues/90 [1]
Signed-off-by: Kees Cook <kees@kernel.org>
---
 fs/xfs/scrub/trace.h | 3 +--
 fs/xfs/xfs_trace.h   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 39ea651cbb75..46c420f51129 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -980,8 +980,7 @@ TRACE_EVENT(xfile_create,
 		__entry->ino = file_inode(xf->file)->i_ino;
 		path = file_path(xf->file, __entry->pathname, MAXNAMELEN);
 		if (IS_ERR(path))
-			strncpy(__entry->pathname, "(unknown)",
-					sizeof(__entry->pathname));
+			strscpy_pad(__entry->pathname, "(unknown)");
 	),
 	TP_printk("xfino 0x%lx path '%s'",
 		  __entry->ino,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 813e5a9f57eb..9f9fb86097ed 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -5101,8 +5101,7 @@ TRACE_EVENT(xmbuf_create,
 		__entry->ino = file_inode(file)->i_ino;
 		path = file_path(file, __entry->pathname, MAXNAMELEN);
 		if (IS_ERR(path))
-			strncpy(__entry->pathname, "(unknown)",
-					sizeof(__entry->pathname));
+			strscpy_pad(__entry->pathname, "(unknown)");
 	),
 	TP_printk("dev %d:%d xmino 0x%lx path '%s'",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-- 
2.34.1

Re: [PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[Darrick J. Wong]

On Mon, Mar 23, 2026 at 10:22:09AM -0700, Kees Cook wrote:
> Replace the deprecated[1] strncpy() with strscpy_pad() in the
> xfile_create and xmbuf_create tracepoints.
> 
> Both tracepoints use file_path() to resolve a pathname into
> __entry->pathname (a char[MAXNAMELEN] trace ring buffer field). On
> failure, the error path overwrites the buffer with the string literal
> "(unknown)" via strncpy(). The original strncpy() zero-pads the
> remaining 246 bytes (MAXNAMELEN is 256, "(unknown)" is 10 bytes
> including NUL).
> 
> strscpy_pad() preserves this zero-padding, which matters because the
> destination is a trace ring buffer entry: ring buffer slots are not
> zeroed on allocation, and the raw buffer is readable by userspace via
> tracefs. The zero-padding ensures no stale data remains in the
> buffer after the error path overwrites it.
> 
> The source is a 10-byte string literal into a 256-byte destination,
> so there is no behavioral change.
> 
> Link: https://github.com/KSPP/linux/issues/90 [1]
> Signed-off-by: Kees Cook <kees@kernel.org>

Seems fine to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/trace.h | 3 +--
>  fs/xfs/xfs_trace.h   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 39ea651cbb75..46c420f51129 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -980,8 +980,7 @@ TRACE_EVENT(xfile_create,
>  		__entry->ino = file_inode(xf->file)->i_ino;
>  		path = file_path(xf->file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("xfino 0x%lx path '%s'",
>  		  __entry->ino,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 813e5a9f57eb..9f9fb86097ed 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -5101,8 +5101,7 @@ TRACE_EVENT(xmbuf_create,
>  		__entry->ino = file_inode(file)->i_ino;
>  		path = file_path(file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("dev %d:%d xmino 0x%lx path '%s'",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> -- 
> 2.34.1
> 
> 

Re: [PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[Steven Rostedt]

On Mon, 23 Mar 2026 10:22:09 -0700
Kees Cook <kees@kernel.org> wrote:

> Replace the deprecated[1] strncpy() with strscpy_pad() in the
> xfile_create and xmbuf_create tracepoints.
> 
> Both tracepoints use file_path() to resolve a pathname into
> __entry->pathname (a char[MAXNAMELEN] trace ring buffer field). On
> failure, the error path overwrites the buffer with the string literal
> "(unknown)" via strncpy(). The original strncpy() zero-pads the
> remaining 246 bytes (MAXNAMELEN is 256, "(unknown)" is 10 bytes
> including NUL).
> 
> strscpy_pad() preserves this zero-padding, which matters because the
> destination is a trace ring buffer entry: ring buffer slots are not
> zeroed on allocation, and the raw buffer is readable by userspace via
> tracefs. The zero-padding ensures no stale data remains in the
> buffer after the error path overwrites it.
> 
> The source is a 10-byte string literal into a 256-byte destination,
> so there is no behavioral change.

Hmm, seems to be a lot of waste. Note, the ring buffer now zeros out its
data on allocation so there's no need to zero pad anything, at least not
for exposing data. You may expose a previous trace.

Also, why are these using full path size and not just use dynamic array.
That would be much more efficient in both performance and space.

-- Steve


> 
> Link: https://github.com/KSPP/linux/issues/90 [1]
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  fs/xfs/scrub/trace.h | 3 +--
>  fs/xfs/xfs_trace.h   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 39ea651cbb75..46c420f51129 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -980,8 +980,7 @@ TRACE_EVENT(xfile_create,
>  		__entry->ino = file_inode(xf->file)->i_ino;
>  		path = file_path(xf->file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("xfino 0x%lx path '%s'",
>  		  __entry->ino,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 813e5a9f57eb..9f9fb86097ed 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -5101,8 +5101,7 @@ TRACE_EVENT(xmbuf_create,
>  		__entry->ino = file_inode(file)->i_ino;
>  		path = file_path(file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("dev %d:%d xmino 0x%lx path '%s'",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),

Re: [PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[David Laight]

On Mon, 23 Mar 2026 10:22:09 -0700
Kees Cook <kees@kernel.org> wrote:

> Replace the deprecated[1] strncpy() with strscpy_pad() in the
> xfile_create and xmbuf_create tracepoints.
> 
> Both tracepoints use file_path() to resolve a pathname into
> __entry->pathname (a char[MAXNAMELEN] trace ring buffer field). On
> failure, the error path overwrites the buffer with the string literal
> "(unknown)" via strncpy(). The original strncpy() zero-pads the
> remaining 246 bytes (MAXNAMELEN is 256, "(unknown)" is 10 bytes
> including NUL).
> 
> strscpy_pad() preserves this zero-padding, which matters because the
> destination is a trace ring buffer entry: ring buffer slots are not
> zeroed on allocation, and the raw buffer is readable by userspace via
> tracefs. The zero-padding ensures no stale data remains in the
> buffer after the error path overwrites it.

Eh?
AFAICT file_path() doesn't zero pad on success.
Not only that is calls d_path() to do the work and that has the comment:

 * Returns a pointer into the buffer or an error code if the path was
 * too long. Note: Callers should use the returned pointer, not the passed
 * in buffer, to use the name! The implementation often starts at an offset
 * into the buffer, and may leave 0 bytes at the start.

So the code actually looks entirely broken.

	David

> 
> The source is a 10-byte string literal into a 256-byte destination,
> so there is no behavioral change.
> 
> Link: https://github.com/KSPP/linux/issues/90 [1]
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  fs/xfs/scrub/trace.h | 3 +--
>  fs/xfs/xfs_trace.h   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 39ea651cbb75..46c420f51129 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -980,8 +980,7 @@ TRACE_EVENT(xfile_create,
>  		__entry->ino = file_inode(xf->file)->i_ino;
>  		path = file_path(xf->file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("xfino 0x%lx path '%s'",
>  		  __entry->ino,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 813e5a9f57eb..9f9fb86097ed 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -5101,8 +5101,7 @@ TRACE_EVENT(xmbuf_create,
>  		__entry->ino = file_inode(file)->i_ino;
>  		path = file_path(file, __entry->pathname, MAXNAMELEN);
>  		if (IS_ERR(path))
> -			strncpy(__entry->pathname, "(unknown)",
> -					sizeof(__entry->pathname));
> +			strscpy_pad(__entry->pathname, "(unknown)");
>  	),
>  	TP_printk("dev %d:%d xmino 0x%lx path '%s'",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),

Re: [PATCH] xfs: Replace strncpy() with strscpy_pad() in tracepoint error paths

[Darrick J. Wong]

On Mon, Mar 23, 2026 at 03:25:14PM -0400, Steven Rostedt wrote:
> On Mon, 23 Mar 2026 10:22:09 -0700
> Kees Cook <kees@kernel.org> wrote:
> 
> > Replace the deprecated[1] strncpy() with strscpy_pad() in the
> > xfile_create and xmbuf_create tracepoints.
> > 
> > Both tracepoints use file_path() to resolve a pathname into
> > __entry->pathname (a char[MAXNAMELEN] trace ring buffer field). On
> > failure, the error path overwrites the buffer with the string literal
> > "(unknown)" via strncpy(). The original strncpy() zero-pads the
> > remaining 246 bytes (MAXNAMELEN is 256, "(unknown)" is 10 bytes
> > including NUL).
> > 
> > strscpy_pad() preserves this zero-padding, which matters because the
> > destination is a trace ring buffer entry: ring buffer slots are not
> > zeroed on allocation, and the raw buffer is readable by userspace via
> > tracefs. The zero-padding ensures no stale data remains in the
> > buffer after the error path overwrites it.
> > 
> > The source is a 10-byte string literal into a 256-byte destination,
> > so there is no behavioral change.
> 
> Hmm, seems to be a lot of waste. Note, the ring buffer now zeros out its
> data on allocation so there's no need to zero pad anything, at least not
> for exposing data. You may expose a previous trace.
> 
> Also, why are these using full path size and not just use dynamic array.
> That would be much more efficient in both performance and space.

"Darrick was too stupid to figure that out when he wrote it."

Initially the shmem files used by xfile/xmbuf provided a longer
description of exactly what the file was being used for (e.g. "AG 43
rmapbt") but the dynamic string memory allocations were annoying to have
to catch since some of them exceeded 16 bytes (or whatever the
"guaranteed not to fail" limits arenow) and then I was snowed under by a
bunch of static checker people and so I gave up and ripped all that out.

So at this point the description is hardly useful and maybe I'll just
submit a patch to rip out all the file_path tracepoint stuff.

--D

> -- Steve
> 
> 
> > 
> > Link: https://github.com/KSPP/linux/issues/90 [1]
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---
> >  fs/xfs/scrub/trace.h | 3 +--
> >  fs/xfs/xfs_trace.h   | 3 +--
> >  2 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> > index 39ea651cbb75..46c420f51129 100644
> > --- a/fs/xfs/scrub/trace.h
> > +++ b/fs/xfs/scrub/trace.h
> > @@ -980,8 +980,7 @@ TRACE_EVENT(xfile_create,
> >  		__entry->ino = file_inode(xf->file)->i_ino;
> >  		path = file_path(xf->file, __entry->pathname, MAXNAMELEN);
> >  		if (IS_ERR(path))
> > -			strncpy(__entry->pathname, "(unknown)",
> > -					sizeof(__entry->pathname));
> > +			strscpy_pad(__entry->pathname, "(unknown)");
> >  	),
> >  	TP_printk("xfino 0x%lx path '%s'",
> >  		  __entry->ino,
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 813e5a9f57eb..9f9fb86097ed 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -5101,8 +5101,7 @@ TRACE_EVENT(xmbuf_create,
> >  		__entry->ino = file_inode(file)->i_ino;
> >  		path = file_path(file, __entry->pathname, MAXNAMELEN);
> >  		if (IS_ERR(path))
> > -			strncpy(__entry->pathname, "(unknown)",
> > -					sizeof(__entry->pathname));
> > +			strscpy_pad(__entry->pathname, "(unknown)");
> >  	),
> >  	TP_printk("dev %d:%d xmino 0x%lx path '%s'",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> 
>