* [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT
@ 2025-12-19 14:22 Mickaël Salaün
2025-12-19 14:26 ` Mickaël Salaün
2025-12-23 23:11 ` Günther Noack
0 siblings, 2 replies; 3+ messages in thread
From: Mickaël Salaün @ 2025-12-19 14:22 UTC (permalink / raw)
To: linux-security-module; +Cc: Mickaël Salaün
Until now, each landlock_request struct were allocated on the stack, even
if not really used, because is_access_to_paths_allowed() unconditionally
modified the passed references. Even if the changed landlock_request
variables are not used, the compiler is not smart enough to detect this
case.
To avoid this issue, explicitly disable the related code when
CONFIG_AUDIT is not set, which enables elision of log_request_parent*
and associated caller's stack variables thanks to dead code elimination.
This makes it possible to reduce the stack frame by 192 bytes for the
path_link and path_rename hooks, and by 96 bytes for most other
filesystem hooks.
Here is a summary of scripts/checkstack.pl before and after this change
when CONFIG_AUDIT is disabled:
Function Old size New size Diff
----------------------------------------------------------
current_check_refer_path 384 208 -176
current_check_access_path 192 112 -80
hook_file_open 208 128 -80
is_access_to_paths_allowed 240 224 -16
Also, add extra pointer checks to be more future-proof.
Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/fs.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index fe794875ad46..722f950307f6 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -939,7 +939,12 @@ static bool is_access_to_paths_allowed(
}
path_put(&walker_path);
- if (!allowed_parent1) {
+ /*
+ * Check CONFIG_AUDIT to enable elision of log_request_parent* and
+ * associated caller's stack variables thanks to dead code elimination.
+ */
+#ifdef CONFIG_AUDIT
+ if (!allowed_parent1 && log_request_parent1) {
log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent1->audit.u.path = *path;
@@ -949,7 +954,7 @@ static bool is_access_to_paths_allowed(
ARRAY_SIZE(*layer_masks_parent1);
}
- if (!allowed_parent2) {
+ if (!allowed_parent2 && log_request_parent2) {
log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent2->audit.u.path = *path;
@@ -958,6 +963,8 @@ static bool is_access_to_paths_allowed(
log_request_parent2->layer_masks_size =
ARRAY_SIZE(*layer_masks_parent2);
}
+#endif /* CONFIG_AUDIT */
+
return allowed_parent1 && allowed_parent2;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT
2025-12-19 14:22 [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT Mickaël Salaün
@ 2025-12-19 14:26 ` Mickaël Salaün
2025-12-23 23:11 ` Günther Noack
1 sibling, 0 replies; 3+ messages in thread
From: Mickaël Salaün @ 2025-12-19 14:26 UTC (permalink / raw)
To: linux-security-module; +Cc: Günther Noack, Justin Suess
On Fri, Dec 19, 2025 at 03:22:59PM +0100, Mickaël Salaün wrote:
> Until now, each landlock_request struct were allocated on the stack, even
> if not really used, because is_access_to_paths_allowed() unconditionally
> modified the passed references. Even if the changed landlock_request
> variables are not used, the compiler is not smart enough to detect this
> case.
>
> To avoid this issue, explicitly disable the related code when
> CONFIG_AUDIT is not set, which enables elision of log_request_parent*
> and associated caller's stack variables thanks to dead code elimination.
> This makes it possible to reduce the stack frame by 192 bytes for the
> path_link and path_rename hooks, and by 96 bytes for most other
> filesystem hooks.
>
> Here is a summary of scripts/checkstack.pl before and after this change
> when CONFIG_AUDIT is disabled:
>
> Function Old size New size Diff
> ----------------------------------------------------------
> current_check_refer_path 384 208 -176
> current_check_access_path 192 112 -80
> hook_file_open 208 128 -80
> is_access_to_paths_allowed 240 224 -16
>
> Also, add extra pointer checks to be more future-proof.
>
Reported-by: Tingmao Wang <m@maowtm.org>
Link: https://lore.kernel.org/r/eb86863b-53b0-460b-b223-84dd31d765b9@maowtm.org
> Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/fs.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index fe794875ad46..722f950307f6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -939,7 +939,12 @@ static bool is_access_to_paths_allowed(
> }
> path_put(&walker_path);
>
> - if (!allowed_parent1) {
> + /*
> + * Check CONFIG_AUDIT to enable elision of log_request_parent* and
> + * associated caller's stack variables thanks to dead code elimination.
> + */
> +#ifdef CONFIG_AUDIT
> + if (!allowed_parent1 && log_request_parent1) {
> log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent1->audit.u.path = *path;
> @@ -949,7 +954,7 @@ static bool is_access_to_paths_allowed(
> ARRAY_SIZE(*layer_masks_parent1);
> }
>
> - if (!allowed_parent2) {
> + if (!allowed_parent2 && log_request_parent2) {
> log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> @@ -958,6 +963,8 @@ static bool is_access_to_paths_allowed(
> log_request_parent2->layer_masks_size =
> ARRAY_SIZE(*layer_masks_parent2);
> }
> +#endif /* CONFIG_AUDIT */
> +
> return allowed_parent1 && allowed_parent2;
> }
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT
2025-12-19 14:22 [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT Mickaël Salaün
2025-12-19 14:26 ` Mickaël Salaün
@ 2025-12-23 23:11 ` Günther Noack
1 sibling, 0 replies; 3+ messages in thread
From: Günther Noack @ 2025-12-23 23:11 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: linux-security-module
On Fri, Dec 19, 2025 at 03:22:59PM +0100, Mickaël Salaün wrote:
> Until now, each landlock_request struct were allocated on the stack, even
> if not really used, because is_access_to_paths_allowed() unconditionally
> modified the passed references. Even if the changed landlock_request
> variables are not used, the compiler is not smart enough to detect this
> case.
>
> To avoid this issue, explicitly disable the related code when
> CONFIG_AUDIT is not set, which enables elision of log_request_parent*
> and associated caller's stack variables thanks to dead code elimination.
> This makes it possible to reduce the stack frame by 192 bytes for the
> path_link and path_rename hooks, and by 96 bytes for most other
> filesystem hooks.
>
> Here is a summary of scripts/checkstack.pl before and after this change
> when CONFIG_AUDIT is disabled:
>
> Function Old size New size Diff
> ----------------------------------------------------------
> current_check_refer_path 384 208 -176
> current_check_access_path 192 112 -80
> hook_file_open 208 128 -80
> is_access_to_paths_allowed 240 224 -16
>
> Also, add extra pointer checks to be more future-proof.
>
> Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/fs.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index fe794875ad46..722f950307f6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -939,7 +939,12 @@ static bool is_access_to_paths_allowed(
> }
> path_put(&walker_path);
>
> - if (!allowed_parent1) {
> + /*
> + * Check CONFIG_AUDIT to enable elision of log_request_parent* and
> + * associated caller's stack variables thanks to dead code elimination.
> + */
> +#ifdef CONFIG_AUDIT
> + if (!allowed_parent1 && log_request_parent1) {
> log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent1->audit.u.path = *path;
> @@ -949,7 +954,7 @@ static bool is_access_to_paths_allowed(
> ARRAY_SIZE(*layer_masks_parent1);
> }
>
> - if (!allowed_parent2) {
> + if (!allowed_parent2 && log_request_parent2) {
> log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> @@ -958,6 +963,8 @@ static bool is_access_to_paths_allowed(
> log_request_parent2->layer_masks_size =
> ARRAY_SIZE(*layer_masks_parent2);
> }
> +#endif /* CONFIG_AUDIT */
> +
> return allowed_parent1 && allowed_parent2;
> }
>
> --
> 2.52.0
>
Thanks, this looks good to me!
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-23 23:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19 14:22 [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT Mickaël Salaün
2025-12-19 14:26 ` Mickaël Salaün
2025-12-23 23:11 ` Günther Noack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).