Re: [PATCH 1/4] pstore: convert to the new mount API

[Kees Cook <kees@kernel.org>]

On February 5, 2025 1:34:29 PM PST, Eric Sandeen <sandeen@redhat.com> wrote:
>Convert the pstore filesystem to the new mount API.

Thanks for doing this!

> [...]
>+static const struct fs_parameter_spec pstore_param_spec[] = {
>+	fsparam_u32	("kmsg_bytes",	Opt_kmsg_bytes),
>+	{}

I am reminded to change the global to u32 (ulong is way too big), and fix the pstore_set_kmsg_bytes() prototype to be u32 not int. :)

> [...]
>+	/* pstore has historically ignored invalid kmsg_bytes param */
>+	if (opt < 0)
>+		return 0;

Seems like a warning would be at least user-friendly. ;) I can add that later.

> [...]
>@@ -431,19 +434,33 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
> 		return -ENOMEM;
> 
> 	scoped_guard(mutex, &pstore_sb_lock)
>-		pstore_sb = sb;
>+	pstore_sb = sb;

Shouldn't scoped_guard() induce a indent?

> 
> 	pstore_get_records(0);
> 
> 	return 0;
> }
> 
>-static struct dentry *pstore_mount(struct file_system_type *fs_type,
>-	int flags, const char *dev_name, void *data)
>+static int pstore_get_tree(struct fs_context *fc)
>+{
>+	if (fc->root)
>+		return pstore_reconfigure(fc);

I need to double check that changing kmsg_size out from under an active pstore won't cause problems, but it's probably okay. (Honestly I've been wanting to deprecate it as a mount option -- it really should just be a module param, but that's a separate task.)

Reviewed-by: Kees Cook <kees@kernel.org>

(Is it easier to take this via fs or via pstore?)

-Kees

-- 
Kees Cook