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

[David Laight <david.laight.linux@gmail.com>]

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),