From: Ross Lagerwall <ross.lagerwall@citrix.com>
The intention of lockdown mode is to prevent attacks from a rogue dom0
userspace from compromising the system. Lockdown mode can be controlled by a
Kconfig option and a command-line parameter. It is also enabled automatically
when Secure Boot is enabled and it cannot be disabled in that case.
If enabled from the command-line then it is required to be first in the
list otherwise Xen may process some insecure parameters before reaching
the lockdown parameter.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Kevin Lampis <kevin.lampis@cloud.com>
---
Changes in v2:
- Remove custom command line parsing
- Print warning if lockdown is not first on command line
---
xen/arch/x86/setup.c | 1 +
xen/common/Kconfig | 8 ++++++
xen/common/Makefile | 1 +
xen/common/kernel.c | 7 +++++
xen/common/lockdown.c | 54 ++++++++++++++++++++++++++++++++++++++
xen/include/xen/lockdown.h | 11 ++++++++
6 files changed, 82 insertions(+)
create mode 100644 xen/common/lockdown.c
create mode 100644 xen/include/xen/lockdown.h
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 1f5cb67bd0..efeed5eafc 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -15,6 +15,7 @@
#include <xen/kexec.h>
#include <xen/keyhandler.h>
#include <xen/lib.h>
+#include <xen/lockdown.h>
#include <xen/multiboot.h>
#include <xen/nodemask.h>
#include <xen/numa.h>
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0951d4c2f2..33cd669110 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -587,4 +587,12 @@ config BUDDY_ALLOCATOR_SIZE
Amount of memory reserved for the buddy allocator to serve Xen heap,
working alongside the colored one.
+config LOCKDOWN_DEFAULT
+ bool "Enable lockdown mode by default"
+ default n
+ help
+ Lockdown mode prevents attacks from a rogue dom0 userspace from
+ compromising the system. This is automatically enabled when Secure
+ Boot is enabled.
+
endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 98f0873056..b00a8a925a 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_KEXEC) += kexec.o
obj-$(CONFIG_KEXEC) += kimage.o
obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
obj-$(CONFIG_LLC_COLORING) += llc-coloring.o
+obj-y += lockdown.o
obj-$(CONFIG_VM_EVENT) += mem_access.o
obj-y += memory.o
obj-y += multicall.o
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 8b63ca55f1..7280da987d 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -14,6 +14,7 @@
#include <xen/guest_access.h>
#include <xen/hypercall.h>
#include <xen/hypfs.h>
+#include <xen/lockdown.h>
#include <xsm/xsm.h>
#include <asm/current.h>
#include <public/version.h>
@@ -199,6 +200,8 @@ static int parse_params(const char *cmdline, const struct kernel_param *start,
printk("parameter \"%s\" unknown!\n", key);
final_rc = -EINVAL;
}
+
+ lockdown_clear_first_flag();
}
return final_rc;
@@ -216,6 +219,9 @@ static void __init _cmdline_parse(const char *cmdline)
*/
void __init cmdline_parse(const char *cmdline)
{
+ /* Call this early since it affects command-line parsing */
+ lockdown_init(cmdline);
+
if ( opt_builtin_cmdline[0] )
{
printk("Built-in command line: %s\n", opt_builtin_cmdline);
@@ -227,6 +233,7 @@ void __init cmdline_parse(const char *cmdline)
return;
safe_strcpy(saved_cmdline, cmdline);
+ lockdown_set_first_flag();
_cmdline_parse(cmdline);
#endif
}
diff --git a/xen/common/lockdown.c b/xen/common/lockdown.c
new file mode 100644
index 0000000000..84eabe9c83
--- /dev/null
+++ b/xen/common/lockdown.c
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <xen/efi.h>
+#include <xen/lockdown.h>
+#include <xen/param.h>
+
+#define FIRST_ARG_FLAG 2
+
+static int __ro_after_init lockdown = IS_ENABLED(CONFIG_LOCKDOWN_DEFAULT);
+
+void __init lockdown_set_first_flag(void)
+{
+ lockdown |= FIRST_ARG_FLAG;
+}
+
+void __init lockdown_clear_first_flag(void)
+{
+ lockdown &= ~FIRST_ARG_FLAG;
+}
+
+static int __init parse_lockdown_opt(const char *s)
+{
+ if ( strncmp("no", s, 2) == 0 )
+ if ( efi_secure_boot )
+ printk("lockdown can't be disabled because Xen booted in Secure Boot mode\n");
+ else
+ lockdown = 0;
+ else
+ {
+ if ( !(lockdown & FIRST_ARG_FLAG) )
+ printk("lockdown was not the first argument, unsafe arguments may have been already processed\n");
+
+ lockdown = 1;
+ }
+
+ return 0;
+}
+custom_param("lockdown", parse_lockdown_opt);
+
+bool is_locked_down(void)
+{
+ return lockdown & ~FIRST_ARG_FLAG;
+}
+
+void __init lockdown_init(const char *cmdline)
+{
+ if ( efi_secure_boot )
+ {
+ printk("Enabling lockdown mode because Secure Boot is enabled\n");
+ lockdown = 1;
+ }
+
+ printk("Lockdown mode is %s\n", is_locked_down() ? "enabled" : "disabled");
+}
diff --git a/xen/include/xen/lockdown.h b/xen/include/xen/lockdown.h
new file mode 100644
index 0000000000..6ae97f9d5f
--- /dev/null
+++ b/xen/include/xen/lockdown.h
@@ -0,0 +1,11 @@
+#ifndef XEN__LOCKDOWN_H
+#define XEN__LOCKDOWN_H
+
+#include <xen/types.h>
+
+void lockdown_set_first_flag(void);
+void lockdown_clear_first_flag(void);
+bool is_locked_down(void);
+void lockdown_init(const char *cmdline);
+
+#endif /* XEN__LOCKDOWN_H */
--
2.42.0On 02/06/2025 2:46 pm, Kevin Lampis wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 1f5cb67bd0..efeed5eafc 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -15,6 +15,7 @@
> #include <xen/kexec.h>
> #include <xen/keyhandler.h>
> #include <xen/lib.h>
> +#include <xen/lockdown.h>
> #include <xen/multiboot.h>
> #include <xen/nodemask.h>
> #include <xen/numa.h>
As the only modification to setup.c, this hunk surely isn't in the right
patch.
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 0951d4c2f2..33cd669110 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -587,4 +587,12 @@ config BUDDY_ALLOCATOR_SIZE
> Amount of memory reserved for the buddy allocator to serve Xen heap,
> working alongside the colored one.
>
> +config LOCKDOWN_DEFAULT
> + bool "Enable lockdown mode by default"
> + default n
default n is redundant. Please drop it.
> + help
> + Lockdown mode prevents attacks from a rogue dom0 userspace from
> + compromising the system. This is automatically enabled when Secure
> + Boot is enabled.
It's more than just rogue dom0 userspace. But, are we using lockdown
mode for anything more than just cmdline filtering?
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 8b63ca55f1..7280da987d 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -199,6 +200,8 @@ static int parse_params(const char *cmdline, const struct kernel_param *start,
> printk("parameter \"%s\" unknown!\n", key);
> final_rc = -EINVAL;
> }
> +
> + lockdown_clear_first_flag();
You're calling an __init function from a non-__init one.
I've submitted
https://lore.kernel.org/xen-devel/20250603125215.2716132-1-andrew.cooper3@citrix.com/T/#u
to fix it.
But honestly, given 3 function calls for trivial operations but with
complicated semantics, I'm not sure splitting out lockdown.c out of
kernel.c is a good move.
> }
>
> return final_rc;
> @@ -216,6 +219,9 @@ static void __init _cmdline_parse(const char *cmdline)
> */
> void __init cmdline_parse(const char *cmdline)
> {
> + /* Call this early since it affects command-line parsing */
> + lockdown_init(cmdline);
> +
> if ( opt_builtin_cmdline[0] )
> {
> printk("Built-in command line: %s\n", opt_builtin_cmdline);
Even from just this hunk, the positioning looks suspicious. Existing
UEFI-SB support in Xen relies on the builtin cmdline to provide
configuration, seeing as it ends up part of the signed whole.
Beyond that, I don't see what the fuss is over argument order. The only
case where it matters is if Xen defaults to 0 and a user explicitly
wants to activate lockdown mode on the cmdline, at which point warning
them that their order of arguments was problematic is a) a problem in an
of itself, and b) unworkable when e.g. placeholder is in use.
> @@ -227,6 +233,7 @@ void __init cmdline_parse(const char *cmdline)
> return;
>
> safe_strcpy(saved_cmdline, cmdline);
> + lockdown_set_first_flag();
> _cmdline_parse(cmdline);
> #endif
> }
> diff --git a/xen/common/lockdown.c b/xen/common/lockdown.c
> new file mode 100644
> index 0000000000..84eabe9c83
> --- /dev/null
> +++ b/xen/common/lockdown.c
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <xen/efi.h>
> +#include <xen/lockdown.h>
> +#include <xen/param.h>
> +
> +#define FIRST_ARG_FLAG 2
> +
> +static int __ro_after_init lockdown = IS_ENABLED(CONFIG_LOCKDOWN_DEFAULT);
> +
> +void __init lockdown_set_first_flag(void)
> +{
> + lockdown |= FIRST_ARG_FLAG;
> +}
> +
> +void __init lockdown_clear_first_flag(void)
> +{
> + lockdown &= ~FIRST_ARG_FLAG;
> +}
> +
> +static int __init parse_lockdown_opt(const char *s)
You need a cf_check attribute too. This only doesn't explode in XenRT
because it runs before activating CET, but it will fail in CI.
> +{
> + if ( strncmp("no", s, 2) == 0 )
> + if ( efi_secure_boot )
> + printk("lockdown can't be disabled because Xen booted in Secure Boot mode\n");
> + else
> + lockdown = 0;
Braces please. This is dangerously close to being a buggy expression.
> + else
> + {
> + if ( !(lockdown & FIRST_ARG_FLAG) )
> + printk("lockdown was not the first argument, unsafe arguments may have been already processed\n");
> +
> + lockdown = 1;
> + }
> +
> + return 0;
> +}
> +custom_param("lockdown", parse_lockdown_opt);
> +
> +bool is_locked_down(void)
> +{
> + return lockdown & ~FIRST_ARG_FLAG;
> +}
> +
> +void __init lockdown_init(const char *cmdline)
> +{
> + if ( efi_secure_boot )
> + {
> + printk("Enabling lockdown mode because Secure Boot is enabled\n");
> + lockdown = 1;
> + }
This wants setting by init_secure_boot_mode(). Nothing good can come of
there being a window where efi_secure_boot is set but lockdown is not.
Why is there a cmdline parameter? It doesn't appear to be used.
> +
> + printk("Lockdown mode is %s\n", is_locked_down() ? "enabled" : "disabled");
> +}
~Andrew
On Tue, Jun 3, 2025 at 5:29 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 02/06/2025 2:46 pm, Kevin Lampis wrote: > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > > index 1f5cb67bd0..efeed5eafc 100644 > > --- a/xen/arch/x86/setup.c > > +++ b/xen/arch/x86/setup.c > > @@ -15,6 +15,7 @@ > > #include <xen/kexec.h> > > #include <xen/keyhandler.h> > > #include <xen/lib.h> > > +#include <xen/lockdown.h> > > #include <xen/multiboot.h> > > #include <xen/nodemask.h> > > #include <xen/numa.h> > > As the only modification to setup.c, this hunk surely isn't in the right > patch. > > > diff --git a/xen/common/Kconfig b/xen/common/Kconfig > > index 0951d4c2f2..33cd669110 100644 > > --- a/xen/common/Kconfig > > +++ b/xen/common/Kconfig > > @@ -587,4 +587,12 @@ config BUDDY_ALLOCATOR_SIZE > > Amount of memory reserved for the buddy allocator to serve Xen heap, > > working alongside the colored one. > > > > +config LOCKDOWN_DEFAULT > > + bool "Enable lockdown mode by default" > > + default n > > default n is redundant. Please drop it. > > > + help > > + Lockdown mode prevents attacks from a rogue dom0 userspace from > > + compromising the system. This is automatically enabled when Secure > > + Boot is enabled. > > It's more than just rogue dom0 userspace. But, are we using lockdown > mode for anything more than just cmdline filtering? Not as part of this series, but it is expected that lockdown mode will eventually be tied into certain other functionality. E.g. requiring live patches to be signed when it is enabled. Ross
On Mon, Jun 02, 2025 at 02:46:55PM +0100, Kevin Lampis wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> The intention of lockdown mode is to prevent attacks from a rogue dom0
> userspace from compromising the system. Lockdown mode can be controlled by a
> Kconfig option and a command-line parameter. It is also enabled automatically
> when Secure Boot is enabled and it cannot be disabled in that case.
>
> If enabled from the command-line then it is required to be first in the
> list otherwise Xen may process some insecure parameters before reaching
> the lockdown parameter.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> Signed-off-by: Kevin Lampis <kevin.lampis@cloud.com>
> ---
> Changes in v2:
> - Remove custom command line parsing
> - Print warning if lockdown is not first on command line
> ---
...
> diff --git a/xen/common/lockdown.c b/xen/common/lockdown.c
> new file mode 100644
> index 0000000000..84eabe9c83
> --- /dev/null
> +++ b/xen/common/lockdown.c
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <xen/efi.h>
> +#include <xen/lockdown.h>
> +#include <xen/param.h>
> +
> +#define FIRST_ARG_FLAG 2
> +
> +static int __ro_after_init lockdown = IS_ENABLED(CONFIG_LOCKDOWN_DEFAULT);
> +
> +void __init lockdown_set_first_flag(void)
> +{
> + lockdown |= FIRST_ARG_FLAG;
> +}
> +
> +void __init lockdown_clear_first_flag(void)
> +{
> + lockdown &= ~FIRST_ARG_FLAG;
> +}
> +
> +static int __init parse_lockdown_opt(const char *s)
> +{
> + if ( strncmp("no", s, 2) == 0 )
This is rather inconsistent with other bool options. I think you want to
use parse_bool() here.
> + if ( efi_secure_boot )
> + printk("lockdown can't be disabled because Xen booted in Secure Boot mode\n");
> + else
> + lockdown = 0;
> + else
> + {
> + if ( !(lockdown & FIRST_ARG_FLAG) )
> + printk("lockdown was not the first argument, unsafe arguments may have been already processed\n");
> +
> + lockdown = 1;
> + }
> +
> + return 0;
> +}
> +custom_param("lockdown", parse_lockdown_opt);
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
On Mon, Jun 2, 2025 at 3:20 PM Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote: > > This is rather inconsistent with other bool options. I think you want to > use parse_bool() here. Thank you. I will use that instead.
© 2016 - 2025 Red Hat, Inc.