Re: [PATCH v4 1/1] exec: seal system mappings

[Matthew Wilcox]

On Mon, Nov 25, 2024 at 08:20:21PM +0000, jeffxu@xxxxxxxxxxxx wrote:
> +/*
> + * Kernel cmdline override for CONFIG_SEAL_SYSTEM_MAPPINGS
> + */
> +enum seal_system_mappings_type {
> +	SEAL_SYSTEM_MAPPINGS_DISABLED,
> +	SEAL_SYSTEM_MAPPINGS_ENABLED
> +};
> +
> +static enum seal_system_mappings_type seal_system_mappings_v __ro_after_init =
> +	IS_ENABLED(CONFIG_SEAL_SYSTEM_MAPPINGS) ? SEAL_SYSTEM_MAPPINGS_ENABLED :
> +	SEAL_SYSTEM_MAPPINGS_DISABLED;
> +
> +static const struct constant_table value_table_sys_mapping[] __initconst = {
> +	{ "no", SEAL_SYSTEM_MAPPINGS_DISABLED},
> +	{ "yes", SEAL_SYSTEM_MAPPINGS_ENABLED},
> +	{ }
> +};
> +
> +static int __init early_seal_system_mappings_override(char *buf)
> +{
> +	if (!buf)
> +		return -EINVAL;
> +
> +	seal_system_mappings_v = lookup_constant(value_table_sys_mapping,
> +			buf, seal_system_mappings_v);
> +	return 0;
> +}
> +
> +early_param("exec.seal_system_mappings", early_seal_system_mappings_override);

Are you paid by the line?  This all seems ridiculously overcomplicated.
Look at (first example I found) kgdbwait:

static int __init opt_kgdb_wait(char *str)
{
        kgdb_break_asap = 1;

        kdb_init(KDB_INIT_EARLY);
        if (kgdb_io_module_registered &&
            IS_ENABLED(CONFIG_ARCH_HAS_EARLY_DEBUG))
                kgdb_initial_breakpoint();

        return 0;
}
early_param("kgdbwait", opt_kgdb_wait);

I don't understand why you've created a new 'exec' namespace, and why
this feature fits in 'exec'.  That seems like an implementation detail.
I'd lose the "exec." prefix.