lkml.org 
[lkml]   [2020]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 033/101] proc: Use new_inode not new_inode_pseudo
    Date
    From: Eric W. Biederman <ebiederm@xmission.com>

    commit ef1548adada51a2f32ed7faef50aa465e1b4c5da upstream.

    Recently syzbot reported that unmounting proc when there is an ongoing
    inotify watch on the root directory of proc could result in a use
    after free when the watch is removed after the unmount of proc
    when the watcher exits.

    Commit 69879c01a0c3 ("proc: Remove the now unnecessary internal mount
    of proc") made it easier to unmount proc and allowed syzbot to see the
    problem, but looking at the code it has been around for a long time.

    Looking at the code the fsnotify watch should have been removed by
    fsnotify_sb_delete in generic_shutdown_super. Unfortunately the inode
    was allocated with new_inode_pseudo instead of new_inode so the inode
    was not on the sb->s_inodes list. Which prevented
    fsnotify_unmount_inodes from finding the inode and removing the watch
    as well as made it so the "VFS: Busy inodes after unmount" warning
    could not find the inodes to warn about them.

    Make all of the inodes in proc visible to generic_shutdown_super,
    and fsnotify_sb_delete by using new_inode instead of new_inode_pseudo.
    The only functional difference is that new_inode places the inodes
    on the sb->s_inodes list.

    I wrote a small test program and I can verify that without changes it
    can trigger this issue, and by replacing new_inode_pseudo with
    new_inode the issues goes away.

    Cc: stable@vger.kernel.org
    Link: https://lkml.kernel.org/r/000000000000d788c905a7dfa3f4@google.com
    Reported-by: syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com
    Fixes: 0097875bd415 ("proc: Implement /proc/thread-self to point at the directory of the current thread")
    Fixes: 021ada7dff22 ("procfs: switch /proc/self away from proc_dir_entry")
    Fixes: 51f0885e5415 ("vfs,proc: guarantee unique inodes in /proc")
    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/proc/inode.c | 2 +-
    fs/proc/self.c | 2 +-
    fs/proc/thread_self.c | 2 +-
    3 files changed, 3 insertions(+), 3 deletions(-)

    --- a/fs/proc/inode.c
    +++ b/fs/proc/inode.c
    @@ -415,7 +415,7 @@ const struct inode_operations proc_link_

    struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
    {
    - struct inode *inode = new_inode_pseudo(sb);
    + struct inode *inode = new_inode(sb);

    if (inode) {
    inode->i_ino = de->low_ino;
    --- a/fs/proc/self.c
    +++ b/fs/proc/self.c
    @@ -51,7 +51,7 @@ int proc_setup_self(struct super_block *
    mutex_lock(&root_inode->i_mutex);
    self = d_alloc_name(s->s_root, "self");
    if (self) {
    - struct inode *inode = new_inode_pseudo(s);
    + struct inode *inode = new_inode(s);
    if (inode) {
    inode->i_ino = self_inum;
    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
    --- a/fs/proc/thread_self.c
    +++ b/fs/proc/thread_self.c
    @@ -52,7 +52,7 @@ int proc_setup_thread_self(struct super_
    mutex_lock(&root_inode->i_mutex);
    thread_self = d_alloc_name(s->s_root, "thread-self");
    if (thread_self) {
    - struct inode *inode = new_inode_pseudo(s);
    + struct inode *inode = new_inode(s);
    if (inode) {
    inode->i_ino = thread_self_inum;
    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;

    \
     
     \ /
      Last update: 2020-06-19 18:54    [W:3.346 / U:0.504 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site