Re: [PATCH v4 14/19] selftests/liveupdate: Test which tmpfs files can be preserved

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] selftests/liveupdate: leak of mkdtemp() directory in
  preserve_tmpfs_setup()
--

commit a1db86a2ad704903057c01aec8aee9743875c417
Author: David Matlack <dmatlack@google.com>

selftests/liveupdate: Test which tmpfs files can be preserved

This commit adds selftests verifying which tmpfs files can be preserved by
liveupdate and checking that a preserved file's size remains frozen until it
is unpreserved.

> diff --git a/tools/testing/selftests/liveupdate/liveupdate.c b/tools/testing/selftests/liveupdate/liveupdate.c
> index 4c4272167635b..3d943b5672e6b 100644
> --- a/tools/testing/selftests/liveupdate/liveupdate.c
> +++ b/tools/testing/selftests/liveupdate/liveupdate.c
[ ... ]
> @@ -574,4 +583,298 @@ TEST_F(liveupdate_device, preserve_memfd_with_arg)
>  	ASSERT_EQ(close(mem_fd2), 0);
>  }
>  
> +#define NOBODY 65534
> +
> +FIXTURE(preserve_tmpfs) {
> +	char dir[32];
> +	int luo_fd;
> +	int session_fd;
> +	bool mounted;
> +};
> +
> +FIXTURE_SETUP(preserve_tmpfs)
> +{
> +	self->luo_fd = -1;
> +	self->session_fd = -1;
> +	self->mounted = false;
> +
> +	self->luo_fd = open(LIVEUPDATE_DEV, O_RDWR);
> +	if (self->luo_fd < 0 && errno == ENOENT)
> +		SKIP(return, "%s does not exist", LIVEUPDATE_DEV);
> +	ASSERT_GE(self->luo_fd, 0);
> +
> +	self->session_fd = luo_create_session(self->luo_fd, "preserve-tmpfs");
> +	ASSERT_GE(self->session_fd, 0);
> +
> +	strcpy(self->dir, "/tmp/luo-tmpfs-XXXXXX");
> +	ASSERT_NE(mkdtemp(self->dir), NULL);
> +	if (mount("tmpfs", self->dir, "tmpfs", 0, "size=1M"))
> +		SKIP(return, "Failed to mount tmpfs: %s", strerror(errno));
> +	self->mounted = true;
> +	ASSERT_EQ(chdir(self->dir), 0);
> +}
> +
> +FIXTURE_TEARDOWN(preserve_tmpfs)
> +{
> +	if (self->session_fd >= 0)
> +		close(self->session_fd);
> +	if (self->luo_fd >= 0)
> +		close(self->luo_fd);
> +	if (chdir("/"))
> +		return;
> +	if (self->mounted)
> +		umount2(self->dir, MNT_DETACH);
> +	rmdir(self->dir);
> +}

[Severity: Low]
Does FIXTURE_SETUP(preserve_tmpfs) leak the self->dir directory in /tmp when
mount() fails?

When FIXTURE_SETUP() exits via SKIP(), kselftest_harness terminates the test
child before clearing *_metadata->no_teardown, so FIXTURE_TEARDOWN() does not
run:

tools/testing/selftests/kselftest_harness.h:__TEST_F_IMPL() {
    ...
			fixture_name##_setup(_metadata, self, variant->data); \
			/* Let setup failure terminate early. */ \
			if (_metadata->exit_code) \
				_exit(0); \
			*_metadata->no_teardown = false;
    ...
}

Calling rmdir(self->dir) before SKIP() when mount() fails avoids leaving the
temporary directories behind in /tmp.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111002358.1842967-1-dmatlack@google.com?part=14