From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
Al Viro <viro@zeniv.linux.org.uk>
Cc: Ben Woodard <woodard@redhat.com>,
James Bottomley <James.Bottomley@HansenPartnership.com>,
Jim Foraker <foraker1@llnl.gov>,
Kees Cook <keescook@chromium.org>,
Travis Gummels <tgummels@redhat.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/5] exec: binfmt_misc: don't nullify Node->dentry in kill_node()
Date: Fri, 22 Sep 2017 16:36:41 +0200 [thread overview]
Message-ID: <20170922143641.GA17210@redhat.com> (raw)
In-Reply-To: <20170922143619.GA17179@redhat.com>
kill_node() nullifies/checks Node->dentry to avoid double free. This
complicates the next changes and this is very confusing:
- we do not need to check dentry != NULL under entries_lock, kill_node()
is always called under inode_lock(d_inode(root)) and we rely on this
inode_lock() anyway, without this lock the MISC_FMT_OPEN_FILE cleanup
could race with itself.
- if kill_inode() was already called and ->dentry == NULL we should not
even try to close e->interp_file.
We can change bm_entry_write() to simply check !list_empty(list) before
kill_node. Again, we rely on inode_lock(), in particular it saves us from
the race with bm_status_write(), another caller of kill_node().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/binfmt_misc.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index f471809..f4de5ae 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -600,11 +600,7 @@ static void kill_node(Node *e)
struct dentry *dentry;
write_lock(&entries_lock);
- dentry = e->dentry;
- if (dentry) {
- list_del_init(&e->list);
- e->dentry = NULL;
- }
+ list_del_init(&e->list);
write_unlock(&entries_lock);
if ((e->flags & MISC_FMT_OPEN_FILE) && e->interp_file) {
@@ -612,12 +608,11 @@ static void kill_node(Node *e)
e->interp_file = NULL;
}
- if (dentry) {
- drop_nlink(d_inode(dentry));
- d_drop(dentry);
- dput(dentry);
- simple_release_fs(&bm_mnt, &entry_count);
- }
+ dentry = e->dentry;
+ drop_nlink(d_inode(dentry));
+ d_drop(dentry);
+ dput(dentry);
+ simple_release_fs(&bm_mnt, &entry_count);
}
/* /<entry> */
@@ -662,7 +657,8 @@ static ssize_t bm_entry_write(struct file *file, const char __user *buffer,
root = file_inode(file)->i_sb->s_root;
inode_lock(d_inode(root));
- kill_node(e);
+ if (!list_empty(&e->list))
+ kill_node(e);
inode_unlock(d_inode(root));
break;
@@ -791,7 +787,7 @@ static ssize_t bm_status_write(struct file *file, const char __user *buffer,
inode_lock(d_inode(root));
while (!list_empty(&entries))
- kill_node(list_entry(entries.next, Node, list));
+ kill_node(list_first_entry(&entries, Node, list));
inode_unlock(d_inode(root));
break;
--
2.5.0
next prev parent reply other threads:[~2017-09-22 14:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-22 14:36 [PATCH 0/5] exec: binfmt_misc: fix use-after-free, kill iname[BINPRM_BUF_SIZE] Oleg Nesterov
2017-09-22 14:36 ` Oleg Nesterov [this message]
2017-09-22 14:36 ` [PATCH 2/5] exec: binfmt_misc: shift filp_close(interp_file) from kill_node() to bm_evict_inode() Oleg Nesterov
2017-09-22 14:36 ` [PATCH 3/5] exec: binfmt_misc: remove the confusing e->interp_file != NULL checks Oleg Nesterov
2017-09-22 14:36 ` [PATCH 4/5] exec: binfmt_misc: fix race between load_misc_binary() and kill_node() Oleg Nesterov
2017-09-22 14:36 ` [PATCH 5/5] exec: binfmt_misc: kill the onstack iname[BINPRM_BUF_SIZE] array Oleg Nesterov
2017-09-22 15:24 ` [PATCH 0/5] exec: binfmt_misc: fix use-after-free, kill iname[BINPRM_BUF_SIZE] Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170922143641.GA17210@redhat.com \
--to=oleg@redhat.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=akpm@linux-foundation.org \
--cc=foraker1@llnl.gov \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tgummels@redhat.com \
--cc=viro@zeniv.linux.org.uk \
--cc=woodard@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.