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
signature.asc
Description: PGP signature
