strncpy is deprecated and as such we should prefer less ambiguous and more robust string interfaces [1]. There's a lot of manual memory management to get a prefix and name into a string. Let's use an easier to understand and more robust interface in scnprintf() to accomplish the same task while enabling us to check for possible truncation, resulting in a soft warning. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@xxxxxxxxxxxxxxx Signed-off-by: Justin Stitt <justinstitt@xxxxxxxxxx> --- Changes in v2: - use "%.*s" format specifier - use != instead of < to check for truncation (Christoph H.) - Link to v1: https://lore.kernel.org/r/20240405-strncpy-xattr-split2-v1-1-90ab18232407@xxxxxxxxxx --- Tested with https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git but using scripts + image from: https://github.com/tytso/xfstests-bld test results: https://pastebin.com/44bjhpCp (no failures) Split from https://lore.kernel.org/all/20240401-strncpy-fs-xfs-xfs_ioctl-c-v1-1-02b9feb1989b@xxxxxxxxxx/ with feedback from Christoph H. --- fs/xfs/xfs_xattr.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index 364104e1b38a..54e7e7d24ce9 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c @@ -206,6 +206,7 @@ __xfs_xattr_put_listent( { char *offset; int arraytop; + size_t combined_len, actual_len; if (context->count < 0 || context->seen_enough) return; @@ -220,11 +221,16 @@ __xfs_xattr_put_listent( return; } offset = context->buffer + context->count; - memcpy(offset, prefix, prefix_len); - offset += prefix_len; - strncpy(offset, (char *)name, namelen); /* real name */ - offset += namelen; - *offset = '\0'; + + combined_len = prefix_len + namelen; + + /* plus one byte for \0 */ + actual_len = scnprintf(offset, combined_len + 1, "%.*s%.*s", + prefix_len, prefix, namelen, name); + + if (actual_len != combined_len) + xfs_warn(context->dp->i_mount, + "cannot completely copy context buffer resulting in truncation"); compute_size: context->count += prefix_len + namelen + 1; --- base-commit: c85af715cac0a951eea97393378e84bb49384734 change-id: 20240405-strncpy-xattr-split2-0a3aff0c6a20 Best regards, -- Justin Stitt <justinstitt@xxxxxxxxxx>
