lkml.org 
[lkml]   [2017]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: null dereference in binfmt misc
On 10/09, Tycho Andersen wrote:
> Hi,
>
> It looks like eb23aa031 ("exec: binfmt_misc: remove the confusing
> e->interp_file != NULL checks") uncovered a bug for me (see the trace below,
> which I'm afraid isn't very helpful).

Well, I think this commit uncovered the fact I am stupid, although there is
nothing new. I forgot about iput() in bm_register_write's error paths, it can
be called with MISC_FMT_OPEN_FILE && interp_file == NULL.

I'll try to cleanup bm_register_write() to make this impossible, or perhaps
I will just restore the interp_file != NULL check in evict.

Before that, could you please try the debugging patch below? To ensure you
didn't hit another problem.

Thanks!

Oleg.


--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -589,12 +589,18 @@ static struct inode *bm_get_inode(struct super_block *sb, int mode)
return inode;
}

+#define XXX (void*)1234
+
static void bm_evict_inode(struct inode *inode)
{
Node *e = inode->i_private;

- if (e->flags & MISC_FMT_OPEN_FILE)
- filp_close(e->interp_file, NULL);
+ if (e->flags & MISC_FMT_OPEN_FILE) {
+ if (e->interp_file == XXX)
+ pr_err("register: hit XXX\n");
+ else
+ filp_close(e->interp_file, NULL);
+ }

clear_inode(inode);
kfree(e);
@@ -687,7 +693,6 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
int err = 0;

e = create_entry(buffer, count);
-
if (IS_ERR(e))
return PTR_ERR(e);

@@ -709,6 +714,9 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,

err = simple_pin_fs(&bm_fs_type, &bm_mnt, &entry_count);
if (err) {
+ pr_err("register: failed to pin, f=%d", !!(e->flags & MISC_FMT_OPEN_FILE));
+ if (e->flags & MISC_FMT_OPEN_FILE)
+ e->interp_file = XXX;
iput(inode);
inode = NULL;
goto out2;
@@ -720,7 +728,8 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
f = open_exec(e->interpreter);
if (IS_ERR(f)) {
err = PTR_ERR(f);
- pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
+ pr_err("register: failed to install interpreter\n");
+ e->interp_file = XXX;
simple_release_fs(&bm_mnt, &entry_count);
iput(inode);
inode = NULL;
\
 
 \ /
  Last update: 2017-10-10 13:17    [W:0.044 / U:1.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site