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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z0Tgp4WBPvJiojqG@casper.infradead.org>
Date: Mon, 25 Nov 2024 20:40:07 +0000
From: Matthew Wilcox <willy@...radead.org>
To: jeffxu@...omium.org
Cc: akpm@...ux-foundation.org, keescook@...omium.org, jannh@...gle.com,
	torvalds@...ux-foundation.org, adhemerval.zanella@...aro.org,
	oleg@...hat.com, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org, linux-mm@...ck.org,
	jorgelo@...omium.org, sroettger@...gle.com, ojeda@...nel.org,
	adobriyan@...il.com, anna-maria@...utronix.de, mark.rutland@....com,
	linus.walleij@...aro.org, Jason@...c4.com, deller@....de,
	rdunlap@...radead.org, davem@...emloft.net, hch@....de,
	peterx@...hat.com, hca@...ux.ibm.com, f.fainelli@...il.com,
	gerg@...nel.org, dave.hansen@...ux.intel.com, mingo@...nel.org,
	ardb@...nel.org, Liam.Howlett@...cle.com, mhocko@...e.com,
	42.hyeyoo@...il.com, peterz@...radead.org, ardb@...gle.com,
	enh@...gle.com, rientjes@...gle.com, groeck@...omium.org,
	mpe@...erman.id.au
Subject: Re: [PATCH v4 1/1] exec: seal system mappings

On Mon, Nov 25, 2024 at 08:20:21PM +0000, jeffxu@...omium.org 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.