On Wed, Jan 08, 2025 at 04:43:21PM +0100, Mickaël Salaün wrote:
> Always synchronize access_masked_parent* with access_request_parent*
> according to allowed_parent*. This is required for audit support to be
> able to get back to the reason of denial.
>
> In a rename/link action, instead of always checking a rule two times for
> the same parent directory of the source and the destination files, only
> check it when an action on a child was not already allowed. This also
> enables us to keep consistent allowed_parent* status, which is required
> to get back to the reason of denial.
>
> For internal mount points, only upgrade allowed_parent* to true but do
> not wrongfully set both of them to false otherwise. This is also
> required to get back to the reason of denial.
>
> This does not impact the current behavior but slightly optimize code and
> prepare for audit support that needs to know the exact reason why an
> access was denied.
>
> Cc: Günther Noack <[email protected]>
> Signed-off-by: Mickaël Salaün <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
Pushed in my next tree to simplify next patch series.
> ---
>
> Changes since v2:
> - New patch.
> ---
> security/landlock/fs.c | 44 ++++++++++++++++++++++++++----------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index ddadc465581e..01f9d5e78218 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -854,15 +854,6 @@ static bool is_access_to_paths_allowed(
> child1_is_directory, layer_masks_parent2,
> layer_masks_child2,
> child2_is_directory))) {
> - allowed_parent1 = scope_to_request(
> - access_request_parent1, layer_masks_parent1);
> - allowed_parent2 = scope_to_request(
> - access_request_parent2, layer_masks_parent2);
> -
> - /* Stops when all accesses are granted. */
> - if (allowed_parent1 && allowed_parent2)
> - break;
> -
> /*
> * Now, downgrades the remaining checks from domain
> * handled accesses to requested accesses.
> @@ -870,15 +861,32 @@ static bool is_access_to_paths_allowed(
> is_dom_check = false;
> access_masked_parent1 = access_request_parent1;
> access_masked_parent2 = access_request_parent2;
> +
> + allowed_parent1 =
> + allowed_parent1 ||
> + scope_to_request(access_masked_parent1,
> + layer_masks_parent1);
> + allowed_parent2 =
> + allowed_parent2 ||
> + scope_to_request(access_masked_parent2,
> + layer_masks_parent2);
> +
> + /* Stops when all accesses are granted. */
> + if (allowed_parent1 && allowed_parent2)
> + break;
> }
>
> rule = find_rule(domain, walker_path.dentry);
> - allowed_parent1 = landlock_unmask_layers(
> - rule, access_masked_parent1, layer_masks_parent1,
> - ARRAY_SIZE(*layer_masks_parent1));
> - allowed_parent2 = landlock_unmask_layers(
> - rule, access_masked_parent2, layer_masks_parent2,
> - ARRAY_SIZE(*layer_masks_parent2));
> + allowed_parent1 = allowed_parent1 ||
> + landlock_unmask_layers(
> + rule, access_masked_parent1,
> + layer_masks_parent1,
> + ARRAY_SIZE(*layer_masks_parent1));
> + allowed_parent2 = allowed_parent2 ||
> + landlock_unmask_layers(
> + rule, access_masked_parent2,
> + layer_masks_parent2,
> + ARRAY_SIZE(*layer_masks_parent2));
>
> /* Stops when a rule from each layer grants access. */
> if (allowed_parent1 && allowed_parent2)
> @@ -902,8 +910,10 @@ static bool is_access_to_paths_allowed(
> * access to internal filesystems (e.g. nsfs, which is
> * reachable through /proc/<pid>/ns/<namespace>).
> */
> - allowed_parent1 = allowed_parent2 =
> - !!(walker_path.mnt->mnt_flags & MNT_INTERNAL);
> + if (walker_path.mnt->mnt_flags & MNT_INTERNAL) {
> + allowed_parent1 = true;
> + allowed_parent2 = true;
> + }
> break;
> }
> parent_dentry = dget_parent(walker_path.dentry);
> --
> 2.47.1
>
>
