[PATCH v1] landlock: Use bitops macros in audit code

[Mickaël Salaün]

On Thu, May 08, 2025 at 08:30:46AM +0200, Günther Noack wrote:
> On Wed, May 07, 2025 at 08:54:02PM +0200, Mickaël Salaün wrote:
> > Use the BIT() and BIT_ULL() macros in the new audit code instead of
> > explicit shifts to improve readability.
> > 
> > Cc: Günther Noack <gnoack at google.com>
> > Signed-off-by: Mickaël Salaün <mic at digikod.net>
> > ---
> >  security/landlock/audit.c    | 2 +-
> >  security/landlock/id.c       | 5 +++--
> >  security/landlock/syscalls.c | 3 ++-
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > index 58d5c40d4d0e..c52d079cdb77 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -437,7 +437,7 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> >  		return;
> >  
> >  	/* Checks if the current exec was restricting itself. */
> > -	if (subject->domain_exec & (1 << youngest_layer)) {
> > +	if (subject->domain_exec & BIT(youngest_layer)) {
> >  		/* Ignores denials for the same execution. */
> >  		if (!youngest_denied->log_same_exec)
> >  			return;
> > diff --git a/security/landlock/id.c b/security/landlock/id.c
> > index 11fab9259c15..552272307697 100644
> > --- a/security/landlock/id.c
> > +++ b/security/landlock/id.c
> > @@ -7,6 +7,7 @@
> >  
> >  #include <kunit/test.h>
> >  #include <linux/atomic.h>
> > +#include <linux/bitops.h>
> >  #include <linux/random.h>
> >  #include <linux/spinlock.h>
> >  
> > @@ -25,7 +26,7 @@ static void __init init_id(atomic64_t *const counter, const u32 random_32bits)
> >  	 * Ensures sure 64-bit values are always used by user space (or may
> >  	 * fail with -EOVERFLOW), and makes this testable.
> >  	 */
> > -	init = 1ULL << 32;
> > +	init = BIT_ULL(32);
> >  
> >  	/*
> >  	 * Makes a large (2^32) boot-time value to limit ID collision in logs
> > @@ -105,7 +106,7 @@ static u64 get_id_range(size_t number_of_ids, atomic64_t *const counter,
> >  	 * to get a new ID (e.g. a full landlock_restrict_self() call), and the
> >  	 * cost of draining all available IDs during the system's uptime.
> >  	 */
> > -	random_4bits = random_4bits % (1 << 4);
> > +	random_4bits = random_4bits % BIT(4);
> 
> Optional nit: Might be slightly simpler when written as a bitwise AND:
> 
>   random_4bits = random_4bits & 0b1111;
> 
> (Probably does not make a difference in the compiled code though?)

Yes, it results to the same assembly code.

> 
> And, to also simplify the statement:
> 
>   random_4bits &= 0b1111;

I'll send a v2 with that.

Thanks

> 
> (If you prefer to stick with the modulo, "%=" exists as well, even
> though it's more obscure.)
> 
> >  	step = number_of_ids + random_4bits;
> >  
> >  	/* It is safe to cast a signed atomic to an unsigned value. */
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index b9561e3417ae..33eafb71e4f3 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> > @@ -9,6 +9,7 @@
> >  
> >  #include <asm/current.h>
> >  #include <linux/anon_inodes.h>
> > +#include <linux/bitops.h>
> >  #include <linux/build_bug.h>
> >  #include <linux/capability.h>
> >  #include <linux/cleanup.h>
> > @@ -563,7 +564,7 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> >  	new_llcred->domain = new_dom;
> >  
> >  #ifdef CONFIG_AUDIT
> > -	new_llcred->domain_exec |= 1 << (new_dom->num_layers - 1);
> > +	new_llcred->domain_exec |= BIT(new_dom->num_layers - 1);
> >  #endif /* CONFIG_AUDIT */
> >  
> >  	return commit_creds(new_cred);
> > -- 
> > 2.49.0
> > 
> 
> Signed-off-by: Günther Noack <gnoack3000 at gmail.com>
>