BPF List
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-security-module@vger.kernel.org,
	kernel-team@cloudflare.com, linux-trace-kernel@vger.kernel.org,
	bpf@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v1] landlock: Widen ruleset versions to 64 bits
Date: Tue, 22 Sep 2026 15:30:49 +0200	[thread overview]
Message-ID: <arKDCaXnG-T7To1n@google.com> (raw)
In-Reply-To: <20260922132615.1025945-1-mic@digikod.net>

On Tue, Sep 22, 2026 at 03:26:14PM +0200, Mickaël Salaün wrote:
> Tracepoint consumers use a ruleset ID and version to identify the
> successful landlock_add_rule(2) call prefix used to create a domain.
> LANDLOCK_MAX_NUM_RULES bounds distinct stored rules, not successful
> calls: re-adding already-present rights for an object or port succeeds
> without increasing num_rules.  Because every successful call increments
> the version, these calls can wrap the 32-bit counter and give different
> prefixes the same trace identity.
> 
> Widen the counter and its trace fields to 64 bits so the counter cannot
> wrap in practice, while preserving the successful-call semantics.
> Saturating would alias all subsequent histories, while rejecting a call
> at the limit would change otherwise valid syscall behavior solely for
> trace metadata.
> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Fixes: 63747c94774d ("landlock: Add landlock_add_rule_fs and landlock_add_rule_net tracepoints")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  include/trace/events/landlock.h | 20 ++++++++++----------
>  security/landlock/ruleset.h     |  5 +++--
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h
> index 523ba5ea9870..3a43638c9bc2 100644
> --- a/include/trace/events/landlock.h
> +++ b/include/trace/events/landlock.h
> @@ -344,7 +344,7 @@ TRACE_EVENT(landlock_create_ruleset,
>  
>  	TP_STRUCT__entry(
>  		__field(	u64,		ruleset_id	)
> -		__field(	u32,		ruleset_version	)
> +		__field(	u64,		ruleset_version	)
>  		__field(	access_mask_t,	handled_fs	)
>  		__field(	access_mask_t,	handled_net	)
>  		__field(	access_mask_t,	scoped		)
> @@ -358,7 +358,7 @@ TRACE_EVENT(landlock_create_ruleset,
>  		__entry->scoped		= ruleset->handled_masks.scope;
>  	),
>  
> -	TP_printk("ruleset=%llx.%u handled_fs=%s handled_net=%s scoped=%s",
> +	TP_printk("ruleset=%llx.%llu handled_fs=%s handled_net=%s scoped=%s",
>  		__entry->ruleset_id, __entry->ruleset_version,
>  		__print_flags(__entry->handled_fs, "|", _LANDLOCK_ACCESS_FS_NAMES),
>  		__print_flags(__entry->handled_net, "|", _LANDLOCK_ACCESS_NET_NAMES),
> @@ -384,7 +384,7 @@ TRACE_EVENT(landlock_free_ruleset,
>  
>  	TP_STRUCT__entry(
>  		__field(	u64,		ruleset_id	)
> -		__field(	u32,		ruleset_version	)
> +		__field(	u64,		ruleset_version	)
>  	),
>  
>  	TP_fast_assign(
> @@ -392,7 +392,7 @@ TRACE_EVENT(landlock_free_ruleset,
>  		__entry->ruleset_version = ruleset->version;
>  	),
>  
> -	TP_printk("ruleset=%llx.%u",
> +	TP_printk("ruleset=%llx.%llu",
>  		__entry->ruleset_id, __entry->ruleset_version)
>  );
>  
> @@ -423,7 +423,7 @@ TRACE_EVENT(landlock_add_rule_path_beneath,
>  
>  	TP_STRUCT__entry(
>  		__field(	u64,		ruleset_id	)
> -		__field(	u32,		ruleset_version	)
> +		__field(	u64,		ruleset_version	)
>  		__field(	access_mask_t,	access_rights	)
>  		__field(	dev_t,		dev		)
>  		__field(	ino_t,		ino		)
> @@ -444,7 +444,7 @@ TRACE_EVENT(landlock_add_rule_path_beneath,
>  		__assign_str(pathname);
>  	),
>  
> -	TP_printk("ruleset=%llx.%u access_rights=%s dev=%u:%u ino=%lu path=%s",
> +	TP_printk("ruleset=%llx.%llu access_rights=%s dev=%u:%u ino=%lu path=%s",
>  		__entry->ruleset_id, __entry->ruleset_version,
>  		__print_flags(__entry->access_rights, "|", _LANDLOCK_ACCESS_FS_NAMES),
>  		MAJOR(__entry->dev), MINOR(__entry->dev), __entry->ino,
> @@ -477,7 +477,7 @@ TRACE_EVENT(landlock_add_rule_net_port,
>  
>  	TP_STRUCT__entry(
>  		__field(	u64,		ruleset_id	)
> -		__field(	u32,		ruleset_version	)
> +		__field(	u64,		ruleset_version	)
>  		__field(	access_mask_t,	access_rights	)
>  		__field(	u64,		port		)
>  	),
> @@ -490,7 +490,7 @@ TRACE_EVENT(landlock_add_rule_net_port,
>  		__entry->port		= port;
>  	),
>  
> -	TP_printk("ruleset=%llx.%u access_rights=%s port=%llu",
> +	TP_printk("ruleset=%llx.%llu access_rights=%s port=%llu",
>  		__entry->ruleset_id, __entry->ruleset_version,
>  		__print_flags(__entry->access_rights, "|", _LANDLOCK_ACCESS_NET_NAMES),
>  		__entry->port)
> @@ -526,7 +526,7 @@ TRACE_EVENT(landlock_create_domain,
>  		__field(	u64,		domain_id	)
>  		__field(	u64,		parent_id	)
>  		__field(	u64,		ruleset_id	)
> -		__field(	u32,		ruleset_version	)
> +		__field(	u64,		ruleset_version	)
>  	),
>  
>  	TP_fast_assign(
> @@ -538,7 +538,7 @@ TRACE_EVENT(landlock_create_domain,
>  		__entry->ruleset_version = ruleset->version;
>  	),
>  
> -	TP_printk("domain=%llx parent=%llx ruleset=%llx.%u",
> +	TP_printk("domain=%llx parent=%llx ruleset=%llx.%llu",
>  		__entry->domain_id, __entry->parent_id,
>  		__entry->ruleset_id, __entry->ruleset_version)
>  );
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index b536fa0425b7..cf77f1806a95 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -171,9 +171,10 @@ struct landlock_ruleset {
>  	 * @version: Counter incremented on each successful
>  	 * landlock_add_rule(2), including when it only extends an existing
>  	 * rule's access rights.  Used by tracepoints to correlate a domain with
> -	 * the exact ruleset state it was created from.  Protected by @lock.
> +	 * the exact successful rule history it was created from.  Protected by
> +	 * @lock.
>  	 */
> -	u32 version;
> +	u64 version;
>  	/**
>  	 * @id: Unique identifier for this ruleset, used for tracing.
>  	 */
> -- 
> 2.55.0
> 

Reviewed-by: Günther Noack <gnoack@google.com>

Looks correct.  Good to get this in before 7.3 is finalized.

—Günther

  reply	other threads:[~2026-09-22 13:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22 13:26 [PATCH v1] landlock: Widen ruleset versions to 64 bits Mickaël Salaün
2026-09-22 13:30 ` Günther Noack [this message]
2026-09-22 13:32 ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arKDCaXnG-T7To1n@google.com \
    --to=gnoack@google.com \
    --cc=bpf@vger.kernel.org \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mic@digikod.net \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox