Whamcloud - gitweb
LU-17753 llite: ll_getattr_dentry(): protect access to times 43/54843/5
authorVladimir Saveliev <vladimir.saveliev@hpe.com>
Mon, 13 May 2024 14:57:27 +0000 (17:57 +0300)
committerOleg Drokin <green@whamcloud.com>
Fri, 16 Aug 2024 23:51:30 +0000 (23:51 +0000)
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 <vladimir.saveliev@hpe.com>
Change-Id: Ic94040418da2a5cc5e41458dbf9d4c05244741ff
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54843
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/include/obd_support.h
lustre/llite/file.c
lustre/tests/sanity.sh

index 5b5deda..6b6a627 100644 (file)
@@ -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
index 17bfaa7..14d4815 100644 (file)
@@ -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 &&
index 2a40d8d..018e33e 100755 (executable)
@@ -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 &&