Messages in this thread Patch in this message | | | Date | Wed, 10 Apr 2024 20:14:12 +0000 | Subject | [PATCH v2] xfs: xattr: replace strncpy and check for truncation | From | Justin Stitt <> |
| |
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@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- 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@google.com --- 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@google.com/ 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@google.com>
| |