Re: [PATCH RFC v2 18/23] fs: add umh argument to struct kernel_clone_args

[Jann Horn]

On Fri, Mar 6, 2026 at 12:31 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> Add a umh field to struct kernel_clone_args. When set, copy_fs() copies
> from pid 1's fs_struct instead of the kthread's fs_struct. This ensures
> usermodehelper threads always get init's filesystem state regardless of
> their parent's (kthreadd's) fs.
>
> Usermodehelper threads are not allowed to create mount namespaces
> (CLONE_NEWNS), share filesystem state (CLONE_FS), or be started from
> a non-initial mount namespace. No usermodehelper currently does this so
> we don't need to worry about this restriction.
>
> Set .umh = 1 in user_mode_thread(). At this stage pid 1's fs points to
> rootfs which is the same as kthreadd's fs, so this is functionally
> equivalent.
[...]
> -static int copy_fs(u64 clone_flags, struct task_struct *tsk)
> +static int copy_fs(u64 clone_flags, struct task_struct *tsk, bool umh)
>  {
> -       struct fs_struct *fs = current->fs;
> +       struct fs_struct *fs;
> +
> +       /*
> +        * Usermodehelper may use userspace_init_fs filesystem state but
> +        * they don't get to create mount namespaces, share the
> +        * filesystem state, or be started from a non-initial mount
> +        * namespace.
> +        */
> +       if (umh) {
> +               if (clone_flags & (CLONE_NEWNS | CLONE_FS))
> +                       return -EINVAL;
> +               if (current->nsproxy->mnt_ns != &init_mnt_ns)
> +                       return -EINVAL;
> +               fs = userspace_init_fs;
> +       } else {
> +               fs = current->fs;
> +       }
>
>         VFS_WARN_ON_ONCE(current->fs != current->real_fs);

Should that VFS_WARN_ON_ONCE() be in the else {} block?

I don't know if it could happen that a VFS operation that happens with
overwritten current->fs calls back into firmware loading or such, or
if that is anyway impossible for locking reasons or such...