All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: Christian Brauner <brauner@kernel.org>,
	Paul Moore <paul@paul-moore.com>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	Justin Suess <utilityemal77@gmail.com>,
	Lennart Poettering <lennart@poettering.net>,
	Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>,
	Nicolas Bouchinet <nicolas.bouchinet@oss.cyber.gouv.fr>,
	Shervin Oloumi <enlightened@google.com>,
	Tingmao Wang <m@maowtm.org>,
	kernel-team@cloudflare.com, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [RFC PATCH v1 07/11] selftests/landlock: Drain stale audit records on init
Date: Tue, 24 Mar 2026 14:27:43 +0100	[thread overview]
Message-ID: <acKRT6_wJkISyrLb@google.com> (raw)
In-Reply-To: <20260312100444.2609563-8-mic@digikod.net>

On Thu, Mar 12, 2026 at 11:04:40AM +0100, Mickaël Salaün wrote:
> Non-audit Landlock tests generate audit records as side effects when
> audit_enabled is non-zero (e.g. from boot configuration).  These records
> accumulate in the kernel audit backlog while no audit daemon socket is
> open.  When the next test opens a new netlink socket and registers as
> the audit daemon, the stale backlog is delivered, causing baseline
> record count checks to fail spuriously.
> 
> Fix this by draining all pending records in audit_init() right after
> setting the receive timeout.  The 1-usec SO_RCVTIMEO causes audit_recv()
> to return -EAGAIN once the backlog is empty, naturally terminating the
> drain loop.
> 
> Domain deallocation records are emitted asynchronously from a work
> queue, so they may still arrive after the drain.  Remove records.domain
> == 0 checks from tests where a stale deallocation record from a previous
> test could cause spurious failures.
> 
> Also fix a socket file descriptor leak on error paths in audit_init():
> if audit_set_status() or setsockopt() fails (e.g.  when another audit
> daemon is already registered), close the socket before returning.
> 
> Fix off-by-one checks in matches_log_domain_allocated() and
> matches_log_domain_deallocated() where snprintf() truncation was
> detected with ">" instead of ">=" (snprintf() returns the length
> excluding the NUL terminator, so equality means truncation).
> 
> Cc: Günther Noack <gnoack@google.com>
> Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  tools/testing/selftests/landlock/audit.h      | 29 +++++++++++++++----
>  tools/testing/selftests/landlock/audit_test.c |  2 --
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
> index 44eb433e9666..550acaafcc1e 100644
> --- a/tools/testing/selftests/landlock/audit.h
> +++ b/tools/testing/selftests/landlock/audit.h
> @@ -309,7 +309,7 @@ static int __maybe_unused matches_log_domain_allocated(int audit_fd, pid_t pid,
>  
>  	log_match_len =
>  		snprintf(log_match, sizeof(log_match), log_template, pid);
> -	if (log_match_len > sizeof(log_match))
> +	if (log_match_len >= sizeof(log_match))
>  		return -E2BIG;
>  
>  	return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
> @@ -326,7 +326,7 @@ static int __maybe_unused matches_log_domain_deallocated(
>  
>  	log_match_len = snprintf(log_match, sizeof(log_match), log_template,
>  				 num_denials);
> -	if (log_match_len > sizeof(log_match))
> +	if (log_match_len >= sizeof(log_match))
>  		return -E2BIG;
>  
>  	return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
> @@ -379,19 +379,36 @@ static int audit_init(void)
>  
>  	err = audit_set_status(fd, AUDIT_STATUS_ENABLED, 1);
>  	if (err)
> -		return err;
> +		goto err_close;
>  
>  	err = audit_set_status(fd, AUDIT_STATUS_PID, getpid());
>  	if (err)
> -		return err;
> +		goto err_close;
>  
>  	/* Sets a timeout for negative tests. */
>  	err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &audit_tv_default,
>  			 sizeof(audit_tv_default));
> -	if (err)
> -		return -errno;
> +	if (err) {
> +		err = -errno;
> +		goto err_close;
> +	}
> +
> +	/*
> +	 * Drains stale audit records that accumulated in the kernel backlog
> +	 * while no audit daemon socket was open.  This happens when
> +	 * non-audit Landlock tests create domains or trigger denials while
> +	 * audit_enabled is non-zero (e.g. from boot configuration), or when
> +	 * domain deallocation records arrive asynchronously after a
> +	 * previous test's socket was closed.
> +	 */
> +	while (audit_recv(fd, NULL) == 0)
> +		;
>  
>  	return fd;
> +
> +err_close:
> +	close(fd);
> +	return err;
>  }
>  
>  static int audit_init_filter_exe(struct audit_filter *filter, const char *path)
> diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
> index 46d02d49835a..f92ba6774faa 100644
> --- a/tools/testing/selftests/landlock/audit_test.c
> +++ b/tools/testing/selftests/landlock/audit_test.c
> @@ -412,7 +412,6 @@ TEST_F(audit_flags, signal)
>  		} else {
>  			EXPECT_EQ(1, records.access);
>  		}
> -		EXPECT_EQ(0, records.domain);
>  
>  		/* Updates filter rules to match the drop record. */
>  		set_cap(_metadata, CAP_AUDIT_CONTROL);
> @@ -601,7 +600,6 @@ TEST_F(audit_exec, signal_and_open)
>  	/* Tests that there was no denial until now. */
>  	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
>  	EXPECT_EQ(0, records.access);
> -	EXPECT_EQ(0, records.domain);
>  
>  	/*
>  	 * Wait for the child to do a first denied action by layer1 and
> -- 
> 2.53.0
> 

Ooh, nice catch!  I have definitely stumbled across this bug in the
past (especially when the kernel is compiled with more debugging
options), and I know from Justin that he ran into it as well.
Draining the audit logs before sending a new stimulus for audit
logging looks like a good approach.

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

—Günther

  reply	other threads:[~2026-03-24 13:27 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12 10:04 [RFC PATCH v1 00/11] Landlock: Namespace and capability control Mickaël Salaün
2026-03-12 10:04 ` [RFC PATCH v1 01/11] security: add LSM blob and hooks for namespaces Mickaël Salaün
2026-03-25 12:31   ` Christian Brauner
2026-04-09 16:40     ` Mickaël Salaün
2026-04-10  9:35       ` Christian Brauner
2026-04-22 21:21   ` Günther Noack
2026-04-23  0:19   ` Paul Moore
2026-04-24 18:56     ` Mickaël Salaün
2026-04-24 19:28       ` Paul Moore
2026-04-27 14:57         ` Christian Brauner
2026-04-27 21:46           ` Paul Moore
2026-03-12 10:04 ` [RFC PATCH v1 02/11] security: Add LSM_AUDIT_DATA_NS for namespace audit records Mickaël Salaün
2026-03-25 12:32   ` Christian Brauner
2026-04-01 16:38     ` Mickaël Salaün
2026-04-01 18:48       ` Mickaël Salaün
2026-04-09 13:29         ` Christian Brauner
2026-04-22 21:21   ` Günther Noack
2026-03-12 10:04 ` [RFC PATCH v1 03/11] nsproxy: Add FOR_EACH_NS_TYPE() X-macro and CLONE_NS_ALL Mickaël Salaün
2026-03-25 12:33   ` Christian Brauner
2026-03-25 15:26     ` Mickaël Salaün
2026-03-26 14:22   ` (subset) " Christian Brauner
2026-03-12 10:04 ` [RFC PATCH v1 04/11] landlock: Wrap per-layer access masks in struct layer_rights Mickaël Salaün
2026-04-10  1:45   ` Tingmao Wang
2026-04-22 21:29   ` Günther Noack
2026-03-12 10:04 ` [RFC PATCH v1 05/11] landlock: Enforce namespace entry restrictions Mickaël Salaün
2026-03-29 13:15   ` kernel test robot
2026-04-10  1:45   ` Tingmao Wang
2026-05-08 15:46   ` Günther Noack
2026-03-12 10:04 ` [RFC PATCH v1 06/11] landlock: Enforce capability restrictions Mickaël Salaün
2026-04-22 21:36   ` Günther Noack
2026-05-08 15:54   ` Günther Noack
2026-03-12 10:04 ` [RFC PATCH v1 07/11] selftests/landlock: Drain stale audit records on init Mickaël Salaün
2026-03-24 13:27   ` Günther Noack [this message]
2026-03-12 10:04 ` [RFC PATCH v1 08/11] selftests/landlock: Add namespace restriction tests Mickaël Salaün
2026-03-12 10:04 ` [RFC PATCH v1 09/11] selftests/landlock: Add capability " Mickaël Salaün
2026-03-12 10:04 ` [RFC PATCH v1 10/11] samples/landlock: Add capability and namespace restriction support Mickaël Salaün
2026-04-22 21:20   ` Günther Noack
2026-04-23 13:51     ` Mickaël Salaün
2026-03-12 10:04 ` [RFC PATCH v1 11/11] landlock: Add documentation for capability and namespace restrictions Mickaël Salaün
2026-03-12 14:48   ` Justin Suess
2026-04-23 13:51     ` Mickaël Salaün
2026-04-23 16:01       ` Justin Suess
2026-04-23 16:08         ` Justin Suess
2026-04-22 20:38   ` Günther Noack
2026-04-23 13:52     ` Mickaël Salaün
2026-05-08 15:13       ` Günther Noack
2026-03-25 12:34 ` [RFC PATCH v1 00/11] Landlock: Namespace and capability control Christian Brauner
2026-04-20 15:06 ` Günther Noack
2026-04-21  8:24   ` Mickaël Salaün
2026-04-22 21:16     ` Günther Noack
2026-04-23 13:50       ` Mickaël Salaün

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=acKRT6_wJkISyrLb@google.com \
    --to=gnoack@google.com \
    --cc=brauner@kernel.org \
    --cc=enlightened@google.com \
    --cc=ivanov.mikhail1@huawei-partners.com \
    --cc=kernel-team@cloudflare.com \
    --cc=lennart@poettering.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=m@maowtm.org \
    --cc=mic@digikod.net \
    --cc=nicolas.bouchinet@oss.cyber.gouv.fr \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=utilityemal77@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.