| Message ID | 20190909141908.8903-1-maxiwell@linux.ibm.com |
|---|---|
| Headers | show |
| Series | discover: Check if the kernel image has Ultravisor support | expand |
Hi Maxiwell, > The PPC kernel image has an ELF Note 'namespace' called 'PowerPC' > to store capabilities that indicates if the powerpc kernel binary > knows how to run in an ultravisor-enabled system. > > This patchset enables petitboot to read the ELF structures of > kernel binary using the libelf as low-level support and get the > kernel image ELF Note capabilities. If that image is not compatible > with the system, the boot process is aborted. It's not really up to petitboot to decide whether an image is bootable - that's up to the kexec implementation and/or the kernel to determine. I suspect that we'd need better error reporting for this scenario, but it's not petitboot's job to stop the boot process. You're aware that petitboot is not only for POWER + ultravisor platforms, right? Your proposed patches would seem to break everything but that. Regards, Jeremy
On Tue, Sep 10, 2019 at 08:56:04AM +0800, Jeremy Kerr wrote: Hi Jeremy, Thanks for the review. > Hi Maxiwell, > > > The PPC kernel image has an ELF Note 'namespace' called 'PowerPC' > > to store capabilities that indicates if the powerpc kernel binary > > knows how to run in an ultravisor-enabled system. > > > > This patchset enables petitboot to read the ELF structures of > > kernel binary using the libelf as low-level support and get the > > kernel image ELF Note capabilities. If that image is not compatible > > with the system, the boot process is aborted. > > It's not really up to petitboot to decide whether an image is bootable - > that's up to the kexec implementation and/or the kernel to determine. I > suspect that we'd need better error reporting for this scenario, but > it's not petitboot's job to stop the boot process. > Looking the kexec_load() function, I found the call to the validate_boot_files() function, that check if both signature verification and decryption are valid to keep the boot. I thought that ultravisor validation could work in a similar way. > You're aware that petitboot is not only for POWER + ultravisor > platforms, right? Your proposed patches would seem to break everything > but that. Oh, right. The petitboot must know that the environment is a ultravisor-enabled system to check this capability. So, are you suggesting to not touch in the petitboot code and move this check to kexec or kernel itself? Thanks, > > Regards, > > > Jeremy >
Hi Maxiwell, > Looking the kexec_load() function, I found the call to the > validate_boot_files() function, that check if both signature > verification and decryption are valid to keep the boot. Yeah, that's a bit of a different mechanism - in that case it's up to petitboot to enforce a security policy. > > You're aware that petitboot is not only for POWER + ultravisor > > platforms, right? Your proposed patches would seem to break > > everything but that. > > Oh, right. The petitboot must know that the environment is a > ultravisor-enabled system to check this capability. > > So, are you suggesting to not touch in the petitboot code and move > this check to kexec or kernel itself? I think that what we're trying to provide here is some debug-ability to the UV kernel boot failure. So perhaps it's better for petitboot (or whatever else) to provide a message about a potential future failure, rather than petitboot totally preventing boot here. We'll probably be able to get a better warning message if we do this check in petitboot (eg., it can be appropriately formatted and translated). So, let's keep the check in petitboot, but with a couple of changes: - only run the check when we know we're on an ultravisor platform - have it log a warning that gets to the petitboot UIs (using update_status()), rather than aborting the boot We may want this in powerpc-specific code, which might warrant a platform-specific hook to validate a boot payload, called from boot_process(). I'll leave it to you to pick the best place for that, but let me know if you need a hand navigating the code. Michael - does that work for you? Cheers, Jeremy
Jeremy Kerr <jk@ozlabs.org> writes: > Hi Maxiwell, > >> Looking the kexec_load() function, I found the call to the >> validate_boot_files() function, that check if both signature >> verification and decryption are valid to keep the boot. > > Yeah, that's a bit of a different mechanism - in that case it's up to > petitboot to enforce a security policy. > >> > You're aware that petitboot is not only for POWER + ultravisor >> > platforms, right? Your proposed patches would seem to break >> > everything but that. >> >> Oh, right. The petitboot must know that the environment is a >> ultravisor-enabled system to check this capability. >> >> So, are you suggesting to not touch in the petitboot code and move >> this check to kexec or kernel itself? > > I think that what we're trying to provide here is some debug-ability to > the UV kernel boot failure. So perhaps it's better for petitboot (or > whatever else) to provide a message about a potential future failure, > rather than petitboot totally preventing boot here. > > We'll probably be able to get a better warning message if we do this > check in petitboot (eg., it can be appropriately formatted and > translated). > > So, let's keep the check in petitboot, but with a couple of changes: > > - only run the check when we know we're on an ultravisor platform > > - have it log a warning that gets to the petitboot UIs (using > update_status()), rather than aborting the boot > > We may want this in powerpc-specific code, which might warrant a > platform-specific hook to validate a boot payload, called from > boot_process(). I'll leave it to you to pick the best place for that, > but let me know if you need a hand navigating the code. > > Michael - does that work for you? I think we should block booting by default, we know (or are quite sure) that it's not going to work. But there should then be some mechanism for a user to force the boot if they think they know better than us. cheers
Hi Michael, > I think we should block booting by default, we know (or are quite > sure) that it's not going to work. > > But there should then be some mechanism for a user to force the boot > if they think they know better than us. OK, but we do need that to be more than just an 'are you sure?' dialog - as it needs to also work for automatic boot. So, how about this: - add a NVRAM-based setting to disable pre-boot checks "petitboot,disable-boot-checks" - add a facility to the config UI screen to set the above (but have it default to checks enabled) - have the pre-boot verification stop the boot, but disabled through that setting. How does that sound? Cheers, Jeremy
Jeremy Kerr <jk@ozlabs.org> writes: > Hi Michael, > >> I think we should block booting by default, we know (or are quite >> sure) that it's not going to work. >> >> But there should then be some mechanism for a user to force the boot >> if they think they know better than us. > > OK, but we do need that to be more than just an 'are you sure?' dialog - > as it needs to also work for automatic boot. > > So, how about this: > > - add a NVRAM-based setting to disable pre-boot checks > "petitboot,disable-boot-checks" > > - add a facility to the config UI screen to set the above (but have it > default to checks enabled) > > - have the pre-boot verification stop the boot, but disabled through > that setting. > > How does that sound? Sounds good to me. cheers