From cd7c1407e35f2da2316a7ad67a3f7326694b6d4e Mon Sep 17 00:00:00 2001 From: Vladimir Saveliev Date: Mon, 13 May 2024 17:57:27 +0300 Subject: [PATCH] LU-17753 llite: ll_getattr_dentry(): protect access to times ll_merge_attr() updates inode's timestamps twice. Racing stat may get intermediate values of timestamps. Test to illustrate the issue is added. ll_merge_attr() makes intermediate inode's timestamp update with values from MDS under ll_inode_info->lli_size_mutex. ll_getattr_dentry() should also use the mutex to not return intermediate timestamps. This applies for regular files only. Test-Parameters: testlist=sanity env=ONLY=39u,ONLY_REPEAT=200 HPE-bug-id: LUS-12265 Signed-off-by: Vladimir Saveliev Change-Id: Ic94040418da2a5cc5e41458dbf9d4c05244741ff Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54843 Tested-by: jenkins Reviewed-by: Oleg Drokin --- lustre/include/obd_support.h | 2 ++ lustre/llite/file.c | 23 +++++++++++++++++++++++ lustre/tests/sanity.sh | 17 +++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index 5b5deda..6b6a627 100644 --- a/lustre/include/obd_support.h +++ b/lustre/include/obd_support.h @@ -646,6 +646,8 @@ extern bool obd_enable_health_write; #define OBD_FAIL_LLITE_READ_PAUSE 0x1431 #define OBD_FAIL_LLITE_FAULT_PAUSE 0x1432 #define OBD_FAIL_LLITE_STATAHEAD_PAUSE 0x1433 +#define OBD_FAIL_LLITE_STAT_RACE1 0x1434 +#define OBD_FAIL_LLITE_STAT_RACE2 0x1435 #define OBD_FAIL_FID_INDIR 0x1501 #define OBD_FAIL_FID_INLMA 0x1502 diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 17bfaa7..14d4815 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -1511,6 +1511,13 @@ static int ll_merge_attr_nolock(const struct lu_env *env, struct inode *inode) if (rc != 0) GOTO(out, rc = (rc == -ENODATA ? 0 : rc)); + CFS_RACE(OBD_FAIL_LLITE_STAT_RACE2); + /* + * let a awaken stat thread a chance to get intermediate + * attributes from inode + */ + CFS_FAIL_TIMEOUT(OBD_FAIL_LLITE_STAT_RACE2, 1); + if (atime < attr->cat_atime) atime = attr->cat_atime; @@ -5930,11 +5937,24 @@ fill_attr: else stat->mode = (inode->i_mode & ~S_IFMT) | S_IFLNK; + CFS_FAIL_CHECK_RESET(OBD_FAIL_LLITE_STAT_RACE1, + OBD_FAIL_LLITE_STAT_RACE2); + /* pause to let other stat to do intermediate changes to inode */ + CFS_RACE(OBD_FAIL_LLITE_STAT_RACE2); + + /* + * ll_merge_attr() (in case of regular files) does not update + * inode's timestamps atomically. Protect against intermediate + * values + */ + if (!S_ISDIR(inode->i_mode)) + ll_inode_size_lock(inode); stat->uid = inode->i_uid; stat->gid = inode->i_gid; stat->atime = inode_get_atime(inode); stat->mtime = inode_get_mtime(inode); stat->ctime = inode_get_ctime(inode); + /* stat->blksize is used to tell about preferred IO size */ if (sbi->ll_stat_blksize) stat->blksize = sbi->ll_stat_blksize; @@ -5951,6 +5971,9 @@ fill_attr: stat->size = i_size_read(inode); stat->blocks = inode->i_blocks; + if (!S_ISDIR(inode->i_mode)) + ll_inode_size_unlock(inode); + #if defined(HAVE_USER_NAMESPACE_ARG) || defined(HAVE_INODEOPS_ENHANCED_GETATTR) if (flags & AT_STATX_DONT_SYNC) { if (stat->size == 0 && diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 2a40d8d..018e33e6 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -5556,6 +5556,23 @@ test_39s() { } run_test 39s "relatime is supported" +test_39u() { + touch $DIR/$tfile + sleep 2 + dd if=/dev/zero of=$DIR/$tfile bs=1M count=1 conv=notrunc || + error "dd failed" + + #define OBD_FAIL_LLITE_STAT_RACE1 0x1434 + $LCTL set_param fail_loc=0x80001434 + + local mtimes=($(stat -c "%Y" $DIR/$tfile & + sleep 1; stat -c "%Y" $DIR/$tfile; wait)) + + (( ${mtimes[0]} == ${mtimes[1]} )) || + error "mtime mismatch ${mtimes[0]} != ${mtimes[1]}" +} +run_test 39u "stat race" + test_40() { dd if=/dev/zero of=$DIR/$tfile bs=4096 count=1 $RUNAS $OPENFILE -f O_WRONLY:O_TRUNC $DIR/$tfile && -- 1.8.3.1