| Message ID | 20200421003920.16248-1-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted |
| Commit | f0ff75f2491ba27c04bb1f94e502a2be8fc0e78e |
| Headers | show |
| Series | efi_loader: factor out the common code from efi_transfer_secure_state() | expand |
On 4/21/20 2:39 AM, AKASHI Takahiro wrote: > efi_set_secure_stat() provides the common code for each stat transition > caused by efi_transfer_secure_state(). > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > --- > lib/efi_loader/efi_variable.c | 194 +++++++++++----------------------- > 1 file changed, 64 insertions(+), 130 deletions(-) > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index 0c6d1deb58eb..c275684278a1 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -176,6 +176,59 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name, > const void *data, > bool ro_check); > > +/** > + * efi_set_secure_state - modify secure boot state variables > + * @sec_boot: value of SecureBoot > + * @setup_mode: value of SetupMode > + * @audit_mode: value of AuditMode > + * @deployed_mode: value of DeployedMode > + * > + * Modify secure boot stat-related variables as indicated. > + * > + * Return: EFI_SUCCESS on success, status code (negative) on error According to the UEFI spec: EFI_STATUS Type UINTN So the return value cannot be negative. I will adjust the text when merging. Otherwise: Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > + */ > +static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode, > + int audit_mode, int deployed_mode) > +{ > + u32 attributes; > + efi_status_t ret; > + > + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS | > + READ_ONLY; > + ret = efi_set_variable_internal(L"SecureBoot", > + &efi_global_variable_guid, > + attributes, > + sizeof(sec_boot), &sec_boot, > + false); > + if (ret != EFI_SUCCESS) > + goto err; > + > + ret = efi_set_variable_internal(L"SetupMode", > + &efi_global_variable_guid, > + attributes, > + sizeof(setup_mode), &setup_mode, > + false); > + if (ret != EFI_SUCCESS) > + goto err; > + > + ret = efi_set_variable_internal(L"AuditMode", > + &efi_global_variable_guid, > + attributes, > + sizeof(audit_mode), &audit_mode, > + false); > + if (ret != EFI_SUCCESS) > + goto err; > + > + ret = efi_set_variable_internal(L"DeployedMode", > + &efi_global_variable_guid, > + attributes, > + sizeof(deployed_mode), &deployed_mode, > + false); > +err: > + return ret; > +} > + > /** > * efi_transfer_secure_state - handle a secure boot state transition > * @mode: new state > @@ -188,157 +241,38 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name, > */ > static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) > { > - u32 attributes; > - u8 val; > efi_status_t ret; > > - debug("Secure state from %d to %d\n", efi_secure_mode, mode); > + debug("Switching secure state from %d to %d\n", efi_secure_mode, mode); > > - attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | > - EFI_VARIABLE_RUNTIME_ACCESS; > if (mode == EFI_MODE_DEPLOYED) { > - val = 1; > - ret = efi_set_variable_internal(L"SecureBoot", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"SetupMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"AuditMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 1; > - ret = efi_set_variable_internal(L"DeployedMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > + ret = efi_set_secure_state(1, 0, 0, 1); > if (ret != EFI_SUCCESS) > goto err; > > efi_secure_boot = true; > } else if (mode == EFI_MODE_AUDIT) { > - ret = efi_set_variable_internal(L"PK", > - &efi_global_variable_guid, > - attributes, > - 0, NULL, > - false); > + ret = efi_set_variable_internal( > + L"PK", &efi_global_variable_guid, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS, > + 0, NULL, false); > if (ret != EFI_SUCCESS) > goto err; > - val = 0; > - ret = efi_set_variable_internal(L"SecureBoot", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 1; > - ret = efi_set_variable_internal(L"SetupMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 1; > - ret = efi_set_variable_internal(L"AuditMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"DeployedMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > + > + ret = efi_set_secure_state(0, 1, 1, 0); > if (ret != EFI_SUCCESS) > goto err; > > efi_secure_boot = true; > } else if (mode == EFI_MODE_USER) { > - val = 1; > - ret = efi_set_variable_internal(L"SecureBoot", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"SetupMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"AuditMode", > - &efi_global_variable_guid, > - attributes, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"DeployedMode", > - &efi_global_variable_guid, > - attributes, > - sizeof(val), &val, > - false); > + ret = efi_set_secure_state(1, 0, 0, 0); > if (ret != EFI_SUCCESS) > goto err; > > efi_secure_boot = true; > } else if (mode == EFI_MODE_SETUP) { > - val = 0; > - ret = efi_set_variable_internal(L"SecureBoot", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 1; > - ret = efi_set_variable_internal(L"SetupMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"AuditMode", > - &efi_global_variable_guid, > - attributes, > - sizeof(val), &val, > - false); > - if (ret != EFI_SUCCESS) > - goto err; > - val = 0; > - ret = efi_set_variable_internal(L"DeployedMode", > - &efi_global_variable_guid, > - attributes | READ_ONLY, > - sizeof(val), &val, > - false); > + ret = efi_set_secure_state(0, 1, 0, 0); > if (ret != EFI_SUCCESS) > goto err; > } else { >
On 5/1/20 8:22 AM, Heinrich Schuchardt wrote: > On 4/21/20 2:39 AM, AKASHI Takahiro wrote: >> efi_set_secure_stat() provides the common code for each stat transition >> caused by efi_transfer_secure_state(). >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> >> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >> --- >> lib/efi_loader/efi_variable.c | 194 +++++++++++----------------------- >> 1 file changed, 64 insertions(+), 130 deletions(-) >> >> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c >> index 0c6d1deb58eb..c275684278a1 100644 >> --- a/lib/efi_loader/efi_variable.c >> +++ b/lib/efi_loader/efi_variable.c >> @@ -176,6 +176,59 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name, >> const void *data, >> bool ro_check); >> >> +/** >> + * efi_set_secure_state - modify secure boot state variables >> + * @sec_boot: value of SecureBoot >> + * @setup_mode: value of SetupMode >> + * @audit_mode: value of AuditMode >> + * @deployed_mode: value of DeployedMode >> + * >> + * Modify secure boot stat-related variables as indicated. >> + * >> + * Return: EFI_SUCCESS on success, status code (negative) on error > According to the UEFI spec: > > EFI_STATUS Type UINTN > > So the return value cannot be negative. I will adjust the text when merging. > > Otherwise: > > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > Patch is merged. Thanks Heinrich
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 0c6d1deb58eb..c275684278a1 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -176,6 +176,59 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name, const void *data, bool ro_check); +/** + * efi_set_secure_state - modify secure boot state variables + * @sec_boot: value of SecureBoot + * @setup_mode: value of SetupMode + * @audit_mode: value of AuditMode + * @deployed_mode: value of DeployedMode + * + * Modify secure boot stat-related variables as indicated. + * + * Return: EFI_SUCCESS on success, status code (negative) on error + */ +static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode, + int audit_mode, int deployed_mode) +{ + u32 attributes; + efi_status_t ret; + + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + READ_ONLY; + ret = efi_set_variable_internal(L"SecureBoot", + &efi_global_variable_guid, + attributes, + sizeof(sec_boot), &sec_boot, + false); + if (ret != EFI_SUCCESS) + goto err; + + ret = efi_set_variable_internal(L"SetupMode", + &efi_global_variable_guid, + attributes, + sizeof(setup_mode), &setup_mode, + false); + if (ret != EFI_SUCCESS) + goto err; + + ret = efi_set_variable_internal(L"AuditMode", + &efi_global_variable_guid, + attributes, + sizeof(audit_mode), &audit_mode, + false); + if (ret != EFI_SUCCESS) + goto err; + + ret = efi_set_variable_internal(L"DeployedMode", + &efi_global_variable_guid, + attributes, + sizeof(deployed_mode), &deployed_mode, + false); +err: + return ret; +} + /** * efi_transfer_secure_state - handle a secure boot state transition * @mode: new state @@ -188,157 +241,38 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name, */ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) { - u32 attributes; - u8 val; efi_status_t ret; - debug("Secure state from %d to %d\n", efi_secure_mode, mode); + debug("Switching secure state from %d to %d\n", efi_secure_mode, mode); - attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS; if (mode == EFI_MODE_DEPLOYED) { - val = 1; - ret = efi_set_variable_internal(L"SecureBoot", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"SetupMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"AuditMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 1; - ret = efi_set_variable_internal(L"DeployedMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); + ret = efi_set_secure_state(1, 0, 0, 1); if (ret != EFI_SUCCESS) goto err; efi_secure_boot = true; } else if (mode == EFI_MODE_AUDIT) { - ret = efi_set_variable_internal(L"PK", - &efi_global_variable_guid, - attributes, - 0, NULL, - false); + ret = efi_set_variable_internal( + L"PK", &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + 0, NULL, false); if (ret != EFI_SUCCESS) goto err; - val = 0; - ret = efi_set_variable_internal(L"SecureBoot", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 1; - ret = efi_set_variable_internal(L"SetupMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 1; - ret = efi_set_variable_internal(L"AuditMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"DeployedMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); + + ret = efi_set_secure_state(0, 1, 1, 0); if (ret != EFI_SUCCESS) goto err; efi_secure_boot = true; } else if (mode == EFI_MODE_USER) { - val = 1; - ret = efi_set_variable_internal(L"SecureBoot", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"SetupMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"AuditMode", - &efi_global_variable_guid, - attributes, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"DeployedMode", - &efi_global_variable_guid, - attributes, - sizeof(val), &val, - false); + ret = efi_set_secure_state(1, 0, 0, 0); if (ret != EFI_SUCCESS) goto err; efi_secure_boot = true; } else if (mode == EFI_MODE_SETUP) { - val = 0; - ret = efi_set_variable_internal(L"SecureBoot", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 1; - ret = efi_set_variable_internal(L"SetupMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"AuditMode", - &efi_global_variable_guid, - attributes, - sizeof(val), &val, - false); - if (ret != EFI_SUCCESS) - goto err; - val = 0; - ret = efi_set_variable_internal(L"DeployedMode", - &efi_global_variable_guid, - attributes | READ_ONLY, - sizeof(val), &val, - false); + ret = efi_set_secure_state(0, 1, 0, 0); if (ret != EFI_SUCCESS) goto err; } else {
efi_set_secure_stat() provides the common code for each stat transition caused by efi_transfer_secure_state(). Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- lib/efi_loader/efi_variable.c | 194 +++++++++++----------------------- 1 file changed, 64 insertions(+), 130 deletions(-)