lkml.org 
[lkml]   [2020]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.6 008/194] shmem: fix possible deadlocks on shmlock_user_lock
    Date
    From: Hugh Dickins <hughd@google.com>

    [ Upstream commit ea0dfeb4209b4eab954d6e00ed136bc6b48b380d ]

    Recent commit 71725ed10c40 ("mm: huge tmpfs: try to split_huge_page()
    when punching hole") has allowed syzkaller to probe deeper, uncovering a
    long-standing lockdep issue between the irq-unsafe shmlock_user_lock,
    the irq-safe xa_lock on mapping->i_pages, and shmem inode's info->lock
    which nests inside xa_lock (or tree_lock) since 4.8's shmem_uncharge().

    user_shm_lock(), servicing SysV shmctl(SHM_LOCK), wants
    shmlock_user_lock while its caller shmem_lock() holds info->lock with
    interrupts disabled; but hugetlbfs_file_setup() calls user_shm_lock()
    with interrupts enabled, and might be interrupted by a writeback endio
    wanting xa_lock on i_pages.

    This may not risk an actual deadlock, since shmem inodes do not take
    part in writeback accounting, but there are several easy ways to avoid
    it.

    Requiring interrupts disabled for shmlock_user_lock would be easy, but
    it's a high-level global lock for which that seems inappropriate.
    Instead, recall that the use of info->lock to guard info->flags in
    shmem_lock() dates from pre-3.1 days, when races with SHMEM_PAGEIN and
    SHMEM_TRUNCATE could occur: nowadays it serves no purpose, the only flag
    added or removed is VM_LOCKED itself, and calls to shmem_lock() an inode
    are already serialized by the caller.

    Take info->lock out of the chain and the possibility of deadlock or
    lockdep warning goes away.

    Fixes: 4595ef88d136 ("shmem: make shmem_inode_info::lock irq-safe")
    Reported-by: syzbot+c8a8197c8852f566b9d9@syzkaller.appspotmail.com
    Reported-by: syzbot+40b71e145e73f78f81ad@syzkaller.appspotmail.com
    Signed-off-by: Hugh Dickins <hughd@google.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Acked-by: Yang Shi <yang.shi@linux.alibaba.com>
    Cc: Yang Shi <yang.shi@linux.alibaba.com>
    Link: http://lkml.kernel.org/r/alpine.LSU.2.11.2004161707410.16322@eggly.anvils
    Link: https://lore.kernel.org/lkml/000000000000e5838c05a3152f53@google.com/
    Link: https://lore.kernel.org/lkml/0000000000003712b305a331d3b1@google.com/
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    mm/shmem.c | 7 +++++--
    1 file changed, 5 insertions(+), 2 deletions(-)

    diff --git a/mm/shmem.c b/mm/shmem.c
    index 7406f91f8a528..153d889e32d1d 100644
    --- a/mm/shmem.c
    +++ b/mm/shmem.c
    @@ -2184,7 +2184,11 @@ int shmem_lock(struct file *file, int lock, struct user_struct *user)
    struct shmem_inode_info *info = SHMEM_I(inode);
    int retval = -ENOMEM;

    - spin_lock_irq(&info->lock);
    + /*
    + * What serializes the accesses to info->flags?
    + * ipc_lock_object() when called from shmctl_do_lock(),
    + * no serialization needed when called from shm_destroy().
    + */
    if (lock && !(info->flags & VM_LOCKED)) {
    if (!user_shm_lock(inode->i_size, user))
    goto out_nomem;
    @@ -2199,7 +2203,6 @@ int shmem_lock(struct file *file, int lock, struct user_struct *user)
    retval = 0;

    out_nomem:
    - spin_unlock_irq(&info->lock);
    return retval;
    }

    --
    2.20.1


    \
     
     \ /
      Last update: 2020-05-18 20:02    [W:4.548 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site