Skip to content

Commit 83d58b6

Browse files
xrl1ksacilotto
authored andcommitted
binfmt_misc: fix possible deadlock in bm_register_write
BugLink: https://bugs.launchpad.net/bugs/1920246 commit e7850f4 upstream. There is a deadlock in bm_register_write: First, in the begining of the function, a lock is taken on the binfmt_misc root inode with inode_lock(d_inode(root)). Then, if the user used the MISC_FMT_OPEN_FILE flag, the function will call open_exec on the user-provided interpreter. open_exec will call a path lookup, and if the path lookup process includes the root of binfmt_misc, it will try to take a shared lock on its inode again, but it is already locked, and the code will get stuck in a deadlock To reproduce the bug: $ echo ":iiiii:E::ii::/proc/sys/fs/binfmt_misc/bla:F" > /proc/sys/fs/binfmt_misc/register backtrace of where the lock occurs (#5): 0 schedule () at ./arch/x86/include/asm/current.h:15 1 0xffffffff81b51237 in rwsem_down_read_slowpath (sem=0xffff888003b202e0, count=<optimized out>, state=state@entry=2) at kernel/locking/rwsem.c:992 2 0xffffffff81b5150a in __down_read_common (state=2, sem=<optimized out>) at kernel/locking/rwsem.c:1213 3 __down_read (sem=<optimized out>) at kernel/locking/rwsem.c:1222 4 down_read (sem=<optimized out>) at kernel/locking/rwsem.c:1355 5 0xffffffff811ee22a in inode_lock_shared (inode=<optimized out>) at ./include/linux/fs.h:783 6 open_last_lookups (op=0xffffc9000022fe34, file=0xffff888004098600, nd=0xffffc9000022fd10) at fs/namei.c:3177 7 path_openat (nd=nd@entry=0xffffc9000022fd10, op=op@entry=0xffffc9000022fe34, flags=flags@entry=65) at fs/namei.c:3366 8 0xffffffff811efe1c in do_filp_open (dfd=<optimized out>, pathname=pathname@entry=0xffff8880031b9000, op=op@entry=0xffffc9000022fe34) at fs/namei.c:3396 9 0xffffffff811e493f in do_open_execat (fd=fd@entry=-100, name=name@entry=0xffff8880031b9000, flags=<optimized out>, flags@entry=0) at fs/exec.c:913 10 0xffffffff811e4a92 in open_exec (name=<optimized out>) at fs/exec.c:948 11 0xffffffff8124aa84 in bm_register_write (file=<optimized out>, buffer=<optimized out>, count=19, ppos=<optimized out>) at fs/binfmt_misc.c:682 12 0xffffffff811decd2 in vfs_write (file=file@entry=0xffff888004098500, buf=buf@entry=0xa758d0 ":iiiii:E::ii::i:CF ", count=count@entry=19, pos=pos@entry=0xffffc9000022ff10) at fs/read_write.c:603 13 0xffffffff811defda in ksys_write (fd=<optimized out>, buf=0xa758d0 ":iiiii:E::ii::i:CF ", count=19) at fs/read_write.c:658 14 0xffffffff81b49813 in do_syscall_64 (nr=<optimized out>, regs=0xffffc9000022ff58) at arch/x86/entry/common.c:46 15 0xffffffff81c0007c in entry_SYSCALL_64 () at arch/x86/entry/entry_64.S:120 To solve the issue, the open_exec call is moved to before the write lock is taken by bm_register_write Link: https://lkml.kernel.org/r/[email protected] Fixes: 948b701 ("binfmt_misc: add persistent opened binary handler for containers") Signed-off-by: Lior Ribak <[email protected]> Acked-by: Helge Deller <[email protected]> Cc: Al Viro <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Kamal Mostafa <[email protected]> Signed-off-by: Kelsey Skunberg <[email protected]>
1 parent a106547 commit 83d58b6

File tree

1 file changed

+14
-15
lines changed

1 file changed

+14
-15
lines changed

fs/binfmt_misc.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -696,12 +696,24 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
696696
struct super_block *sb = file_inode(file)->i_sb;
697697
struct dentry *root = sb->s_root, *dentry;
698698
int err = 0;
699+
struct file *f = NULL;
699700

700701
e = create_entry(buffer, count);
701702

702703
if (IS_ERR(e))
703704
return PTR_ERR(e);
704705

706+
if (e->flags & MISC_FMT_OPEN_FILE) {
707+
f = open_exec(e->interpreter);
708+
if (IS_ERR(f)) {
709+
pr_notice("register: failed to install interpreter file %s\n",
710+
e->interpreter);
711+
kfree(e);
712+
return PTR_ERR(f);
713+
}
714+
e->interp_file = f;
715+
}
716+
705717
inode_lock(d_inode(root));
706718
dentry = lookup_one_len(e->name, root, strlen(e->name));
707719
err = PTR_ERR(dentry);
@@ -725,21 +737,6 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
725737
goto out2;
726738
}
727739

728-
if (e->flags & MISC_FMT_OPEN_FILE) {
729-
struct file *f;
730-
731-
f = open_exec(e->interpreter);
732-
if (IS_ERR(f)) {
733-
err = PTR_ERR(f);
734-
pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
735-
simple_release_fs(&bm_mnt, &entry_count);
736-
iput(inode);
737-
inode = NULL;
738-
goto out2;
739-
}
740-
e->interp_file = f;
741-
}
742-
743740
e->dentry = dget(dentry);
744741
inode->i_private = e;
745742
inode->i_fop = &bm_entry_operations;
@@ -756,6 +753,8 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
756753
inode_unlock(d_inode(root));
757754

758755
if (err) {
756+
if (f)
757+
filp_close(f, NULL);
759758
kfree(e);
760759
return err;
761760
}

0 commit comments

Comments
 (0)