9p: use inode->i_lock to protect i_size_write() under 32-bit

commit 5e3cc1ee1405a7eb3487ed24f786dec01b4cbe1f upstream.

Use inode->i_lock to protect i_size_write(), else i_size_read() in
generic_fillattr() may loop infinitely in read_seqcount_begin() when
multiple processes invoke v9fs_vfs_getattr() or v9fs_vfs_getattr_dotl()
simultaneously under 32-bit SMP environment, and a soft lockup will be
triggered as show below:

  watchdog: BUG: soft lockup - CPU#5 stuck for 22s! [stat:2217]
  Modules linked in:
  CPU: 5 PID: 2217 Comm: stat Not tainted 5.0.0-rc1-00005-g7f702faf5a9e #4
  Hardware name: Generic DT based system
  PC is at generic_fillattr+0x104/0x108
  LR is at 0xec497f00
  pc : [<802b8898>]    lr : [<ec497f00>]    psr: 200c0013
  sp : ec497e20  ip : ed608030  fp : ec497e3c
  r10: 00000000  r9 : ec497f00  r8 : ed608030
  r7 : ec497ebc  r6 : ec497f00  r5 : ee5c1550  r4 : ee005780
  r3 : 0000052d  r2 : 00000000  r1 : ec497f00  r0 : ed608030
  Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
  Control: 10c5387d  Table: ac48006a  DAC: 00000051
  CPU: 5 PID: 2217 Comm: stat Not tainted 5.0.0-rc1-00005-g7f702faf5a9e #4
  Hardware name: Generic DT based system
  Backtrace:
  [<8010d974>] (dump_backtrace) from [<8010dc88>] (show_stack+0x20/0x24)
  [<8010dc68>] (show_stack) from [<80a1d194>] (dump_stack+0xb0/0xdc)
  [<80a1d0e4>] (dump_stack) from [<80109f34>] (show_regs+0x1c/0x20)
  [<80109f18>] (show_regs) from [<801d0a80>] (watchdog_timer_fn+0x280/0x2f8)
  [<801d0800>] (watchdog_timer_fn) from [<80198658>] (__hrtimer_run_queues+0x18c/0x380)
  [<801984cc>] (__hrtimer_run_queues) from [<80198e60>] (hrtimer_run_queues+0xb8/0xf0)
  [<80198da8>] (hrtimer_run_queues) from [<801973e8>] (run_local_timers+0x28/0x64)
  [<801973c0>] (run_local_timers) from [<80197460>] (update_process_times+0x3c/0x6c)
  [<80197424>] (update_process_times) from [<801ab2b8>] (tick_nohz_handler+0xe0/0x1bc)
  [<801ab1d8>] (tick_nohz_handler) from [<80843050>] (arch_timer_handler_virt+0x38/0x48)
  [<80843018>] (arch_timer_handler_virt) from [<80180a64>] (handle_percpu_devid_irq+0x8c/0x240)
  [<801809d8>] (handle_percpu_devid_irq) from [<8017ac20>] (generic_handle_irq+0x34/0x44)
  [<8017abec>] (generic_handle_irq) from [<8017b344>] (__handle_domain_irq+0x6c/0xc4)
  [<8017b2d8>] (__handle_domain_irq) from [<801022e0>] (gic_handle_irq+0x4c/0x88)
  [<80102294>] (gic_handle_irq) from [<80101a30>] (__irq_svc+0x70/0x98)
  [<802b8794>] (generic_fillattr) from [<8056b284>] (v9fs_vfs_getattr_dotl+0x74/0xa4)
  [<8056b210>] (v9fs_vfs_getattr_dotl) from [<802b8904>] (vfs_getattr_nosec+0x68/0x7c)
  [<802b889c>] (vfs_getattr_nosec) from [<802b895c>] (vfs_getattr+0x44/0x48)
  [<802b8918>] (vfs_getattr) from [<802b8a74>] (vfs_statx+0x9c/0xec)
  [<802b89d8>] (vfs_statx) from [<802b9428>] (sys_lstat64+0x48/0x78)
  [<802b93e0>] (sys_lstat64) from [<80101000>] (ret_fast_syscall+0x0/0x28)

[dominique.martinet@cea.fr: updated comment to not refer to a function
in another subsystem]
Link: http://lkml.kernel.org/r/20190124063514.8571-2-houtao1@huawei.com
Cc: stable@vger.kernel.org
Fixes: 7549ae3e81cc ("9p: Use the i_size_[read, write]() macros instead of using inode->i_size directly.")
Reported-by: Xing Gaopeng <xingaopeng@huawei.com>
Signed-off-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
index 5a0db6dec8d1fd4ad2673a03188a910606ae42b3..aaee1e6584e65240995e7f78c408cb257420997f 100644 (file)
--- a/fs/9p/v9fs_vfs.h
+++ b/fs/9p/v9fs_vfs.h
@@ -40,6 +40,9 @@
  */
 #define P9_LOCK_TIMEOUT (30*HZ)
 
+/* flags for v9fs_stat2inode() & v9fs_stat2inode_dotl() */
+#define V9FS_STAT2INODE_KEEP_ISIZE 1
+
 extern struct file_system_type v9fs_fs_type;
 extern const struct address_space_operations v9fs_addr_operations;
 extern const struct file_operations v9fs_file_operations;
@@ -61,8 +64,10 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
                    struct inode *inode, umode_t mode, dev_t);
 void v9fs_evict_inode(struct inode *inode);
 ino_t v9fs_qid2ino(struct p9_qid *qid);
-void v9fs_stat2inode(struct p9_wstat *, struct inode *, struct super_block *);
-void v9fs_stat2inode_dotl(struct p9_stat_dotl *, struct inode *);
+void v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
+                     struct super_block *sb, unsigned int flags);
+void v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
+                          unsigned int flags);
 int v9fs_dir_release(struct inode *inode, struct file *filp);
 int v9fs_file_open(struct inode *inode, struct file *file);
 void v9fs_inode2stat(struct inode *inode, struct p9_wstat *stat);
@@ -83,4 +88,18 @@ static inline void v9fs_invalidate_inode_attr(struct inode *inode)
 }
 
 int v9fs_open_to_dotl_flags(int flags);
+
+static inline void v9fs_i_size_write(struct inode *inode, loff_t i_size)
+{
+       /*
+        * 32-bit need the lock, concurrent updates could break the
+        * sequences and make i_size_read() loop forever.
+        * 64-bit updates are atomic and can skip the locking.
+        */
+       if (sizeof(i_size) > sizeof(long))
+               spin_lock(&inode->i_lock);
+       i_size_write(inode, i_size);
+       if (sizeof(i_size) > sizeof(long))
+               spin_unlock(&inode->i_lock);
+}
 #endif

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index 3a2f37ad1f89c8bbdff87d395e3d14de2e3c9c16..af8cac975a7450475f96d43d93c8e496922a3b36 100644 (file)
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -442,7 +442,11 @@ v9fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
                i_size = i_size_read(inode);
                if (iocb->ki_pos > i_size) {
                        inode_add_bytes(inode, iocb->ki_pos - i_size);
-                       i_size_write(inode, iocb->ki_pos);
+                       /*
+                        * Need to serialize against i_size_write() in
+                        * v9fs_stat2inode()
+                        */
+                       v9fs_i_size_write(inode, iocb->ki_pos);
                }
                return retval;
        }

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index bdabb2765d1b3e324d77a9513ee170da18ec6d5e..e88cb25176dc440b53c50325ecfc6a24787889b3 100644 (file)
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -538,7 +538,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
        if (retval)
                goto error;
 
-       v9fs_stat2inode(st, inode, sb);
+       v9fs_stat2inode(st, inode, sb, 0);
        v9fs_cache_inode_get_cookie(inode);
        unlock_new_inode(inode);
        return inode;
@@ -1080,7 +1080,7 @@ v9fs_vfs_getattr(const struct path *path, struct kstat *stat,
        if (IS_ERR(st))
                return PTR_ERR(st);
 
-       v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb);
+       v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb, 0);
        generic_fillattr(d_inode(dentry), stat);
 
        p9stat_free(st);
@@ -1158,12 +1158,13 @@ static int v9fs_vfs_setattr(struct dentry *dentry, struct iattr *iattr)
  * @stat: Plan 9 metadata (mistat) structure
  * @inode: inode to populate
  * @sb: superblock of filesystem
+ * @flags: control flags (e.g. V9FS_STAT2INODE_KEEP_ISIZE)
  *
  */
 
 void
 v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
-       struct super_block *sb)
+                struct super_block *sb, unsigned int flags)
 {
        umode_t mode;
        char ext[32];
@@ -1204,10 +1205,11 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
        mode = p9mode2perm(v9ses, stat);
        mode |= inode->i_mode & ~S_IALLUGO;
        inode->i_mode = mode;
-       i_size_write(inode, stat->length);
 
+       if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
+               v9fs_i_size_write(inode, stat->length);
        /* not real number of blocks, but 512 byte ones ... */
-       inode->i_blocks = (i_size_read(inode) + 512 - 1) >> 9;
+       inode->i_blocks = (stat->length + 512 - 1) >> 9;
        v9inode->cache_validity &= ~V9FS_INO_INVALID_ATTR;
 }
 
@@ -1404,9 +1406,9 @@ int v9fs_refresh_inode(struct p9_fid *fid, struct inode *inode)
 {
        int umode;
        dev_t rdev;
-       loff_t i_size;
        struct p9_wstat *st;
        struct v9fs_session_info *v9ses;
+       unsigned int flags;
 
        v9ses = v9fs_inode2v9ses(inode);
        st = p9_client_stat(fid);
@@ -1419,16 +1421,13 @@ int v9fs_refresh_inode(struct p9_fid *fid, struct inode *inode)
        if ((inode->i_mode & S_IFMT) != (umode & S_IFMT))
                goto out;
 
-       spin_lock(&inode->i_lock);
        /*
         * We don't want to refresh inode->i_size,
         * because we may have cached data
         */
-       i_size = inode->i_size;
-       v9fs_stat2inode(st, inode, inode->i_sb);
-       if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
-               inode->i_size = i_size;
-       spin_unlock(&inode->i_lock);
+       flags = (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) ?
+               V9FS_STAT2INODE_KEEP_ISIZE : 0;
+       v9fs_stat2inode(st, inode, inode->i_sb, flags);
 out:
        p9stat_free(st);
        kfree(st);

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 7f6ae21a27b3ca451d353398b4a0e25cb55a9177..3446ab1f44e7dac74fb8d14747512d068eb1b404 100644 (file)
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -143,7 +143,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
        if (retval)
                goto error;
 
-       v9fs_stat2inode_dotl(st, inode);
+       v9fs_stat2inode_dotl(st, inode, 0);
        v9fs_cache_inode_get_cookie(inode);
        retval = v9fs_get_acl(inode, fid);
        if (retval)
@@ -497,7 +497,7 @@ v9fs_vfs_getattr_dotl(const struct path *path, struct kstat *stat,
        if (IS_ERR(st))
                return PTR_ERR(st);
 
-       v9fs_stat2inode_dotl(st, d_inode(dentry));
+       v9fs_stat2inode_dotl(st, d_inode(dentry), 0);
        generic_fillattr(d_inode(dentry), stat);
        /* Change block size to what the server returned */
        stat->blksize = st->st_blksize;
@@ -608,11 +608,13 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
  * v9fs_stat2inode_dotl - populate an inode structure with stat info
  * @stat: stat structure
  * @inode: inode to populate
+ * @flags: ctrl flags (e.g. V9FS_STAT2INODE_KEEP_ISIZE)
  *
  */
 
 void
-v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
+v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
+                     unsigned int flags)
 {
        umode_t mode;
        struct v9fs_inode *v9inode = V9FS_I(inode);
@@ -632,7 +634,8 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
                mode |= inode->i_mode & ~S_IALLUGO;
                inode->i_mode = mode;
 
-               i_size_write(inode, stat->st_size);
+               if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
+                       v9fs_i_size_write(inode, stat->st_size);
                inode->i_blocks = stat->st_blocks;
        } else {
                if (stat->st_result_mask & P9_STATS_ATIME) {
@@ -662,8 +665,9 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
                }
                if (stat->st_result_mask & P9_STATS_RDEV)
                        inode->i_rdev = new_decode_dev(stat->st_rdev);
-               if (stat->st_result_mask & P9_STATS_SIZE)
-                       i_size_write(inode, stat->st_size);
+               if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE) &&
+                   stat->st_result_mask & P9_STATS_SIZE)
+                       v9fs_i_size_write(inode, stat->st_size);
                if (stat->st_result_mask & P9_STATS_BLOCKS)
                        inode->i_blocks = stat->st_blocks;
        }
@@ -929,9 +933,9 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
 
 int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
 {
-       loff_t i_size;
        struct p9_stat_dotl *st;
        struct v9fs_session_info *v9ses;
+       unsigned int flags;
 
        v9ses = v9fs_inode2v9ses(inode);
        st = p9_client_getattr_dotl(fid, P9_STATS_ALL);
@@ -943,16 +947,13 @@ int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
        if ((inode->i_mode & S_IFMT) != (st->st_mode & S_IFMT))
                goto out;
 
-       spin_lock(&inode->i_lock);
        /*
         * We don't want to refresh inode->i_size,
         * because we may have cached data
         */
-       i_size = inode->i_size;
-       v9fs_stat2inode_dotl(st, inode);
-       if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
-               inode->i_size = i_size;
-       spin_unlock(&inode->i_lock);
+       flags = (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) ?
+               V9FS_STAT2INODE_KEEP_ISIZE : 0;
+       v9fs_stat2inode_dotl(st, inode, flags);
 out:
        kfree(st);
        return 0;

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 8b75463cb2116895b4a44b28bf1faab7f1db8f71..d4400779f6d93e41eaaf363e0b754e3c7556b5c5 100644 (file)
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -172,7 +172,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
                        goto release_sb;
                }
                d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
-               v9fs_stat2inode_dotl(st, d_inode(root));
+               v9fs_stat2inode_dotl(st, d_inode(root), 0);
                kfree(st);
        } else {
                struct p9_wstat *st = NULL;
@@ -183,7 +183,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
                }
 
                d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
-               v9fs_stat2inode(st, d_inode(root), sb);
+               v9fs_stat2inode(st, d_inode(root), sb, 0);
 
                p9stat_free(st);
                kfree(st);