On Mon, 23 Mar 2026 10:22:09 -0700
Kees Cook <[email protected]> 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 <[email protected]>
> ---
> 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),