Re: [PATCH 02/24] Add a SysRq option to lift kernel lockdown

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Wed 2018-04-11 17:24:52, David Howells wrote:
> From: Kyle McMartin <kyle@redhat.com>
> 
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
> 
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: x86@kernel.org

Is that good idea? Magic sysrq was meant for debugging, not for
toggling options like that. Distros are expected to turn it off.

It also works over serial consoles etc, being able to toggle security
options from serial is surprising...

> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -487,6 +487,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
>  	/* x: May be registered on mips for TLB dump */
>  	/* x: May be registered on ppc/powerpc for xmon */
>  	/* x: May be registered on sparc64 for global PMU dump */
> +	/* x: May be registered on x86_64 for disabling secure boot */
>  	NULL,				/* x */

What about x86-32?

> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> +	.handler	= sysrq_handle_lockdown_lift,
> +	.help_msg	= "unSB(x)",
> +	.action_msg	= "Disabling Secure Boot restrictions",
> +	.enable_mask	= SYSRQ_DISABLE_USERSPACE,
> +};

I'd remove secure boot mentions here.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]