Re: [PATCH v2 3/3] Disallow most command-line options when lockdown mode is enabled

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

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

On Mon, Jun 02, 2025 at 02:46:56PM +0100, Kevin Lampis wrote:
> A subset of command-line parameters that are specifically safe to use when
> lockdown mode is enabled are annotated as such.
> 
> These are commonly used parameters which have been audited to ensure they
> cannot be used to undermine the integrity of the system when booted in
> Secure Boot mode.
> 
> Signed-off-by: Kevin Lampis <kevin.lampis@cloud.com>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> Changes in v2:
> - Add more information about the safe parameters
> - Add lockdown section to the command line doc
> ---
>  docs/misc/xen-command-line.pandoc     | 16 +++++++++
>  xen/arch/arm/domain_build.c           |  4 +--
>  xen/arch/x86/acpi/cpu_idle.c          |  2 +-
>  xen/arch/x86/cpu/amd.c                |  2 +-
>  xen/arch/x86/cpu/mcheck/mce.c         |  2 +-
>  xen/arch/x86/cpu/microcode/core.c     |  2 +-
>  xen/arch/x86/dom0_build.c             |  4 +--
>  xen/arch/x86/hvm/hvm.c                |  2 +-
>  xen/arch/x86/irq.c                    |  2 +-
>  xen/arch/x86/nmi.c                    |  2 +-
>  xen/arch/x86/setup.c                  |  2 +-
>  xen/arch/x86/traps.c                  |  2 +-
>  xen/arch/x86/x86_64/mmconfig-shared.c |  2 +-
>  xen/common/domain.c                   |  2 +-
>  xen/common/kernel.c                   | 10 +++++-
>  xen/common/kexec.c                    |  2 +-
>  xen/common/lockdown.c                 |  2 +-
>  xen/common/numa.c                     |  2 +-
>  xen/common/page_alloc.c               |  2 +-
>  xen/common/shutdown.c                 |  2 +-
>  xen/drivers/char/console.c            |  2 +-
>  xen/drivers/char/ns16550.c            |  4 +--
>  xen/drivers/video/vga.c               |  2 +-
>  xen/include/xen/param.h               | 49 +++++++++++++++++++++------
>  24 files changed, 87 insertions(+), 36 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index b0eadd2c5d..7916875f22 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1798,6 +1798,22 @@ immediately. Specifying `0` will disable all testing of illegal lock nesting.
>  
>  This option is available for hypervisors built with CONFIG_DEBUG_LOCKS only.
>  
> +### lockdown
> +> `= <boolean>`
> +
> +> Default: `false`

This belongs to the 2/3 patch, no?

> +
> +The intention of lockdown mode is to prevent attacks from a rogue dom0
> +userspace from compromising the system. It is also enabled automatically
> +when Secure Boot is enabled and it cannot be disabled in that case.
> +
> +After lockdown mode is enabled some unsafe command line options will be
> +ignored by Xen.
> +
> +If enabling lockdown mode via the command line then ensure it is positioned as
> +the first option in the command line string otherwise Xen may process unsafe
> +options before reaching the lockdown parameter.
> +
>  ### loglvl
>  > `= <level>[/<rate-limited level>]` where level is `none | error | warning | info | debug | all`
>  
...
> diff --git a/xen/common/lockdown.c b/xen/common/lockdown.c
> index 84eabe9c83..cd3deeb63e 100644
> --- a/xen/common/lockdown.c
> +++ b/xen/common/lockdown.c
> @@ -35,7 +35,7 @@ static int __init parse_lockdown_opt(const char *s)
>  
>      return 0;
>  }
> -custom_param("lockdown", parse_lockdown_opt);
> +custom_secure_param("lockdown", parse_lockdown_opt);

Is that really a good idea? It means `lockdown=yes lockdown=no` would
still disable it in the end. This may matter more if for example the
`lockdown=yes` part is in the built-in cmdline (possibly with other
integrity protection than UEFI SB).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]