mirror of
https://github.com/linux-msm/laptops-kernel.git
synced 2026-08-13 14:19:53 -07:00
binfmt_misc: don't leak the user namespace when the mount fails
bm_get_tree() takes a reference to the user namespace and hands it to
get_tree_keyed() as the sget key. sget_fc() moves that reference into
sb->s_fs_info and clears fc->s_fs_info, so from that point on the
superblock owns it and bm_free() doesn't see it anymore.
The superblock drops it in ->put_super(). But generic_shutdown_super()
only calls ->put_super() from inside the if (sb->s_root) branch, so
nothing releases it when bm_fill_super() fails:
- The kzalloc_obj() failure leaves s_root NULL and the whole branch is
skipped.
- A simple_fill_super() failure in the file loop leaves s_root set, but
s_op still points at simple_super_operations, which has no
->put_super(). bm_fill_super() installs s_ops only once
simple_fill_super() returned success, and installing it earlier
wouldn't help either because simple_fill_super() overwrites s_op.
Either way vfs_get_super() calls deactivate_locked_super() and the
reference is gone for good. binfmt_misc mounts are available in a user
namespace and both the inode and the dentry cache are SLAB_ACCOUNT, so
an unprivileged caller under a tight memory cgroup can fail
simple_fill_super() on demand and leak one user namespace per attempt.
Drop the reference in ->kill_sb() instead, which runs unconditionally,
the same way nfsd and rpc_pipefs release their keyed s_fs_info.
That also stops ->put_super() from clearing s_fs_info while the
superblock is still on @fs_supers. generic_shutdown_super() leaves it
there on purpose so that sget_fc() keeps finding it until kill_sb() has
run, but a NULL s_fs_info makes test_keyed_super() miss it, so a
concurrent mount for the same user namespace skips the grab_super()
wait and creates a second superblock for a namespace that is still
being torn down.
Link: https://patch.msgid.link/20260728-work-binfmt_misc-usernsleak-v1-1-dbd8d5e626e7@kernel.org
Fixes: 21ca59b365 ("binfmt_misc: enable sandboxed mounts")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This commit is contained in:
+15
-17
@@ -921,18 +921,9 @@ static const struct file_operations bm_status_operations = {
|
||||
|
||||
/* Superblock handling */
|
||||
|
||||
static void bm_put_super(struct super_block *sb)
|
||||
{
|
||||
struct user_namespace *user_ns = sb->s_fs_info;
|
||||
|
||||
sb->s_fs_info = NULL;
|
||||
put_user_ns(user_ns);
|
||||
}
|
||||
|
||||
static const struct super_operations s_ops = {
|
||||
.statfs = simple_statfs,
|
||||
.evict_inode = bm_evict_inode,
|
||||
.put_super = bm_put_super,
|
||||
};
|
||||
|
||||
static int bm_fill_super(struct super_block *sb, struct fs_context *fc)
|
||||
@@ -990,13 +981,12 @@ static int bm_fill_super(struct super_block *sb, struct fs_context *fc)
|
||||
/*
|
||||
* When the binfmt_misc superblock for this userns is shutdown
|
||||
* ->enabled might have been set to false and we don't reinitialize
|
||||
* ->enabled again in put_super() as someone might already be mounting
|
||||
* binfmt_misc again. It also would be pointless since by the time
|
||||
* ->put_super() is called we know that the binary type list for this
|
||||
* bintfmt_misc mount is empty making load_misc_binary() return
|
||||
* -ENOEXEC independent of whether ->enabled is true. Instead, if
|
||||
* someone mounts binfmt_misc for the first time or again we simply
|
||||
* reset ->enabled to true.
|
||||
* ->enabled again during shutdown as someone might already be mounting
|
||||
* binfmt_misc again. It also would be pointless since by then we know
|
||||
* that the binary type list for this binfmt_misc mount is empty making
|
||||
* load_misc_binary() return -ENOEXEC independent of whether ->enabled
|
||||
* is true. Instead, if someone mounts binfmt_misc for the first time or
|
||||
* again we simply reset ->enabled to true.
|
||||
*/
|
||||
misc->enabled = true;
|
||||
|
||||
@@ -1022,6 +1012,14 @@ static const struct fs_context_operations bm_context_ops = {
|
||||
.get_tree = bm_get_tree,
|
||||
};
|
||||
|
||||
static void bm_kill_sb(struct super_block *sb)
|
||||
{
|
||||
struct user_namespace *user_ns = sb->s_fs_info;
|
||||
|
||||
kill_anon_super(sb);
|
||||
put_user_ns(user_ns);
|
||||
}
|
||||
|
||||
static int bm_init_fs_context(struct fs_context *fc)
|
||||
{
|
||||
fc->ops = &bm_context_ops;
|
||||
@@ -1038,7 +1036,7 @@ static struct file_system_type bm_fs_type = {
|
||||
.name = "binfmt_misc",
|
||||
.init_fs_context = bm_init_fs_context,
|
||||
.fs_flags = FS_USERNS_MOUNT,
|
||||
.kill_sb = kill_anon_super,
|
||||
.kill_sb = bm_kill_sb,
|
||||
};
|
||||
MODULE_ALIAS_FS("binfmt_misc");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user