LU-17400 uapi: Fix incorrect snamelen return value

The sname char array is limited by the struct
changelog_rec.cr_namelen value and has no '\0' character allocated
to it, so strlen() will overrun the char array till it finds the next
'\0' char.

This issue can be seen on the client side when "lfs changelog"
is run and 08RENME record types are present.

Pointer arithmetic was used between sname and name to avoid the
GCC 11 warnings mentioned in 6331eadbd6.

Added Andreas's safety/range check code to changelog_rec_sname.

Fixes: 6331eadbd6 ("LU-15420 uapi: avoid gcc-11 -Werror=stringop-overread")
Signed-off-by: Josh Samuelson <josh@1up.unl.edu>
Change-Id: Ie0817dfdd1d02e06b9399e66f1affaadb9e156c4
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/53624
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Timothy Day <timday@amazon.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: xinliang <xinliang.liu@linaro.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h
index 6b1f909..2f06da2 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lustre_user.h
+++ b/lustre/include/uapi/linux/lustre/lustre_user.h
@@ -2091,15 +2091,17 @@ static inline char *changelog_rec_name(const struct changelog_rec *rec)
 static inline char *changelog_rec_sname(const struct changelog_rec *rec)
 {
        char *str = changelog_rec_name(rec);
+       char *end = str + NAME_MAX; /* NB: NAME_MAX use in CR_MAXSIZE */
 
-       while (*str != '\0')
+       while (*str != '\0' && str <= end)
                str++;
        return str + 1;
 }
 
 static inline __kernel_size_t changelog_rec_snamelen(const struct changelog_rec *rec)
 {
-       return strlen(changelog_rec_sname(rec));
+       return rec->cr_namelen -
+              (changelog_rec_sname(rec) - changelog_rec_name(rec));
 }
 
 /**

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 153a951..d261fbb 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -18738,6 +18738,42 @@ test_160t() {
 }
 run_test 160t "changelog garbage collect on lack of space"
 
+test_160u() { # LU-17400
+       [ $PARALLEL == "yes" ] && skip "skip parallel run"
+       remote_mds_nodsh && skip "remote MDS with nodsh"
+       [ $MDS1_VERSION -ge $(version_code 2.2.0) ] ||
+               skip "Need MDS version at least 2.2.0"
+
+       cd $DIR || error "cd $DIR failed"
+
+       # ensure changelog has a clean view if tests are run multiple times
+       [ -d rename ] && rm -rf rename
+
+       changelog_register || error "changelog_register failed"
+       local cl_user="${CL_USERS[$SINGLEMDS]%% *}"
+
+       changelog_users $SINGLEMDS | grep -q $cl_user ||
+               error "User '$cl_user' not found in changelog_users"
+
+       local longname1=$(str_repeat a 255)
+
+       echo "creating simple directory tree"
+       mkdir -p rename/a || error "create of simple directory tree failed"
+       echo "creating rename/hw file"
+       echo "hello world" > rename/hw || error "create of rename/hw failed"
+       echo "creating very long named file"
+       touch rename/$longname1 || error "create of 'rename/$longname1' failed"
+       echo "move rename/hw to rename/a/a.hw"
+       mv rename/hw rename/a/a.hw || error "mv failed"
+
+       RENME=($(changelog_dump | grep "RENME"))
+       #declare -p RENME # for debugging captured value with indexes
+
+       [[ "${RENME[11]}" == "a.hw" && "${RENME[14]}" == "hw" ]] ||
+               error "changelog rename record type name/sname error"
+}
+run_test 160u "changelog rename record type name and sname strings are correct"
+
 test_161a() {
        [ $PARALLEL == "yes" ] && skip "skip parallel run"