Re: [PATCH v6 5/8] landlock: Convert from sb_mount to granular mount hooks

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Replacing the legacy `sb_mount` and `move_mount` LSM hooks with new granular mount hooks without updating the VFS callers completely drops Landlock's (as well as AppArmor's and SELinux's) mount sandbox protections.
--

--- Patch [5]: [PATCH v6 5/8] landlock: Convert from sb_mount to granular mount hooks ---
commit aefe12302c757bd8116e65f91a6c6d4886925d19
Author: Song Liu <[email protected]>

landlock: Convert from sb_mount to granular mount hooks

This commit replaces Landlock's legacy sb_mount and move_mount hooks
with granular mount hooks (mount_bind, mount_new, etc.). The new hooks
are wired to a common helper that denies all mount operations for
sandboxed processes.

> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c1ecfe2390326..4e6712f972f0a 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c

[ ... ]

> @@ -1950,8 +1978,12 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),
>  
>  	LSM_HOOK_INIT(sb_delete, hook_sb_delete),
> -	LSM_HOOK_INIT(sb_mount, hook_sb_mount),
> -	LSM_HOOK_INIT(move_mount, hook_move_mount),
> +	LSM_HOOK_INIT(mount_bind, hook_mount_bind),
> +	LSM_HOOK_INIT(mount_new, hook_mount_new),
> +	LSM_HOOK_INIT(mount_remount, hook_mount_remount),
> +	LSM_HOOK_INIT(mount_reconfigure, hook_mount_reconfigure),
> +	LSM_HOOK_INIT(mount_change_type, hook_mount_change_type),
> +	LSM_HOOK_INIT(mount_move, hook_move_mount),

[Severity: Critical]
Does this conversion inadvertently bypass Landlock's mount protections?

Looking at path_mount() in fs/namespace.c, the VFS still relies on the legacy
security_sb_mount() API:

fs/namespace.c:path_mount() {
    ...
    ret = security_sb_mount(dev_name, path, type_page, flags, data_page);
    ...
}

And this wrapper strictly evaluates the legacy sb_mount hook list:

security/security.c:security_sb_mount() {
    return call_int_hook(sb_mount, dev_name, path, type, flags, data);
}

Similarly, vfs_move_mount() in fs/namespace.c still calls the legacy
security_move_mount() API.

Because this patch removes sb_mount and move_mount from landlock_hooks, won't
these VFS calls find an empty hook list for Landlock and unconditionally return
0 (success), allowing unprivileged sandboxed processes to perform unauthorized
mounts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5