Messages in this thread Patch in this message | | | Date | Tue, 10 Oct 2017 13:16:47 +0200 | From | Oleg Nesterov <> | Subject | Re: 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;
| |