Re: Subject: [PATCH] Add test for more file systems in landlock - ext4

["Mickaël Salaün" <mic@digikod.net>]

On Tue, Apr 02, 2024 at 01:37:44PM +0530, Saasha Gupta wrote:
> Date: Mon, 2 Apr 2024 19:59:56 +0530
> 
> RE: This patch is now properly preformatted.
> 
> Landlock LSM, a part of the security subsystem, has some tests in place
> for synthetic filesystems such as tmpfs, proc, sysfs, etc. The goal of
> the new issue, and hence this patch is to add tests for non synthetic
> file systems, such as ext4, btrfs, etc

I agree with Julia's review.

> 
> This patch adds tests for the ext4 file system. This includes creation
> of a loop device (test-ext4.img) and formating with mkfs.
> 
> Signed-off-by: Saasha Gupta <saashaa1122@gmail.com>
> ---
>  tools/testing/selftests/landlock/fs_test.c | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 9a6036fbf..b2f2cd5a5 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -4675,6 +4675,14 @@ FIXTURE_VARIANT_ADD(layout3_fs, hostfs) {
>  	.cwd_fs_magic = HOSTFS_SUPER_MAGIC,
>  };
>  
> +/* Add more filesystems */
> +FIXTURE_VARIANT_ADD(layout3_fs, ext4) {
> +	.mnt = {
> +		.type = "ext4",
> +	},
> +	.file_path = TMP_DIR "/dir/file",
> +};
> +
>  FIXTURE_SETUP(layout3_fs)
>  {
>  	struct stat statbuf;
> @@ -4728,6 +4736,63 @@ FIXTURE_SETUP(layout3_fs)
>  		self->has_created_file = true;
>  		clear_cap(_metadata, CAP_DAC_OVERRIDE);
>  	}
> +
> +	/* Create non synthetic file system - ext4 */
> +	if (stat(self->dir_path, &statbuf) != 0) {

dir_path should already exist with previous code right?

> +		pid_t pid = fork();
> +
> +		if (pid == -1) {
> +			perror("Failed to fork");
> +			exit(EXIT_FAILURE);
> +		} else if (pid == 0) {
> +			static const fallocate_argv[] = { "fallocate", "--length",
> +						   "4M", "test-ext4.img",
> +						   NULL };
> +			execvp(fallocate_argv[0], fallocate_argv);

Using system() would makes this much simpler (see net_test.c).

> +			perror("execvp failed");
> +			exit(EXIT_FAILURE);
> +		} else {
> +			int status;
> +
> +			if (waitpid(pid, &status, 0) == -1) {
> +				perror("waitpid failed");
> +				exit(EXIT_FAILURE);
> +			}
> +			if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> +				TH_LOG(stderr,
> +					"Failed to create ext4 filesystem image: fallocate failed\n");
> +				exit(EXIT_FAILURE);
> +			}
> +		}
> +	}
> +
> +	/* Formate and mount non synthetic file system - ext4 */
> +	if (stat("mnt", &statbuf) != 0) {

"mnt" never exists, so this would always run this code...

> +		pid_t pid = fork();
> +
> +		if (pid == -1) {
> +			perror("Failed to fork");
> +			exit(EXIT_FAILURE);
> +		} else if (pid == 0) {
> +			static const mkfs_argv[] = { "mkfs.ext4", "-q",
> +					      "test-ext4.img", "mnt", NULL };
> +			execvp(mkfs_argv[0], mkfs_argv);
> +			perror("execvp failed");
> +			exit(EXIT_FAILURE);
> +		} else {
> +			int status;
> +
> +			if (waitpid(pid, &status, 0) == -1) {
> +				perror("waitpid failed");
> +				exit(EXIT_FAILURE);
> +			}
> +			if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> +				TH_LOG(stderr,
> +					"Failed to format ext4 filesystem image: mkfs.ext4 failed\n");
> +				exit(EXIT_FAILURE);
> +			}
> +		}
> +	}
>  }
>  
>  FIXTURE_TEARDOWN(layout3_fs)
> -- 
> 2.44.0
> 
> 
>