Re: [PATCH 1/2] binfmt_misc: don't let an 'F' entry pin its own instance

> [ ... ]
> 
> Looking at the rest of bm_fill_super() in fs/binfmt_misc.c:
> 
> 	err = simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
> 	if (!err)
> 		sb->s_op = &s_ops;
> 	return err;
> }

This _seems_ legit but I need to double-check.

> [Severity: High]
> This is a pre-existing issue, but does this error path permanently leak the 
> user_namespace reference?
> 
> If simple_fill_super() fails, or if a memory allocation failure occurs 
> earlier in bm_fill_super(), sb->s_root remains NULL. When vfs_get_super() 
> handles the error, generic_shutdown_super() will skip calling 
> s_op->put_super(sb). 
> 
> Since fc->s_fs_info was already cleared during sget_fc(), does this mean 
> the user_namespace reference in sb->s_fs_info is never released?
> 
> Could an unprivileged local user trigger this by intentionally failing 
> userns mounts (e.g., via memory exhaustion) to cause resource exhaustion?
> 
> 
> On a related note regarding pre-existing issues, looking at 
> put_binfmt_handler() in fs/binfmt_misc.c:
> 
> static void put_binfmt_handler(struct binfmt_misc *e)
> {
>         ...
>         if (e->flags & MISC_FMT_OPEN_FILE)
>                 filp_close(e->interp_file, NULL);
>         kfree(e);
> }
> 
> [Severity: High]
> This is a pre-existing issue, but does this teardown path leak the negative 
> i_writecount of the interpreter file?
> 
> When an entry is registered with the 'F' flag, bm_register_write() uses 
> open_exec(), which internally decrements the inode's i_writecount to prevent 
> concurrent writes. While the error path during registration calls 
> exe_file_allow_write_access(), put_binfmt_handler() only calls filp_close(). 
> 
> Since filp_close() does not automatically restore executable write access, 
> would deleting the binfmt_misc entry permanently leave the binary unwritable 
> (returning -ETXTBSY) until reboot?

This is fixed in another patch series of mine.