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

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jun 02, 2025 at 04:22:06PM +0200, Jan Beulich wrote:
> On 02.06.2025 16:16, Marek Marczykowski-Górecki wrote:
> > On Mon, Jun 02, 2025 at 02:46:56PM +0100, Kevin Lampis wrote:
> >> --- 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).
> 
> But having a way to override an earlier "lockdown" by "lockdown=no" is
> intended? E.g. when your xen.cfg has the former, but you don't really
> want that (for, say, an experiment).

Ok, I guess those are conflicting use cases: using "lockdown" option to
restrict what user can set via bootloader menu (even without
secureboot), vs giving the local user full control (developer case). But
in that latter case, maybe you can simply remove the "lockdown" option
instead of adding "lockdown=no" (granted, more work with xen.cfg or
built-in cmdline...) ? 

Anyway, what really matters here is the behavior for UEFI SecureBoot,
and this one is okay. The behavior outside of SB is secondary, and if
that's the intention, I'm okay with the current version too.

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

Attachment: signature.asc
Description: PGP signature