| Message ID | 20231113161031.138304-1-o451686892@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [RESEND,v2] efi_loader: Fix UEFI variable error handling | expand |
On 11/13/23 17:10, Weizhao Ouyang wrote: > Try to catch error the earlier way. > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > --- > lib/efi_loader/efi_var_file.c | 4 +++- > lib/efi_loader/efi_variable.c | 2 -- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c > index 62e071bd83..fe1c462f17 100644 > --- a/lib/efi_loader/efi_var_file.c > +++ b/lib/efi_loader/efi_var_file.c > @@ -192,8 +192,10 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe) > ret = efi_var_mem_ins(var->name, &var->guid, var->attr, > var->length, data, 0, NULL, > var->time); > - if (ret != EFI_SUCCESS) > + if (ret != EFI_SUCCESS) { > log_err("Failed to set EFI variable %ls\n", var->name); > + return ret; This change implies that if a failure occurs, initialization of the EFI sub-system fails and you will not be able to boot via EFI. Such a failure will occur if ubootefi.var contains more variables than fit into the memory buffer. Please, describe why you want to disable U-Boot's EFI sub-system in this case. > + } > } > return EFI_SUCCESS; > } > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index be95ed44e6..2b2ca8c090 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -350,8 +350,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, > > if (var_type == EFI_AUTH_VAR_PK) > ret = efi_init_secure_state(); > - else > - ret = EFI_SUCCESS; This change is ok. Best regards Heinrich > > /* > * Write non-volatile EFI variables to file
On Wed, Nov 15, 2023 at 6:15 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 11/13/23 17:10, Weizhao Ouyang wrote: > > Try to catch error the earlier way. > > > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > > --- > > lib/efi_loader/efi_var_file.c | 4 +++- > > lib/efi_loader/efi_variable.c | 2 -- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c > > index 62e071bd83..fe1c462f17 100644 > > --- a/lib/efi_loader/efi_var_file.c > > +++ b/lib/efi_loader/efi_var_file.c > > @@ -192,8 +192,10 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe) > > ret = efi_var_mem_ins(var->name, &var->guid, var->attr, > > var->length, data, 0, NULL, > > var->time); > > - if (ret != EFI_SUCCESS) > > + if (ret != EFI_SUCCESS) { > > log_err("Failed to set EFI variable %ls\n", var->name); > > + return ret; > > This change implies that if a failure occurs, initialization of the EFI > sub-system fails and you will not be able to boot via EFI. > > Such a failure will occur if ubootefi.var contains more variables than > fit into the memory buffer. > > Please, describe why you want to disable U-Boot's EFI sub-system in this > case. Hi Heinrich, In my case, if a variable wants to persist as a non-volatile variable to ubootefi.var, we should keep the same behavior as runtime variable does. Otherwise the ubootefi.var cannot be trusted. BR, Weizhao > > > + } > > } > > return EFI_SUCCESS; > > } > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > > index be95ed44e6..2b2ca8c090 100644 > > --- a/lib/efi_loader/efi_variable.c > > +++ b/lib/efi_loader/efi_variable.c > > @@ -350,8 +350,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, > > > > if (var_type == EFI_AUTH_VAR_PK) > > ret = efi_init_secure_state(); > > - else > > - ret = EFI_SUCCESS; > > This change is ok. > > Best regards > > Heinrich > > > > > /* > > * Write non-volatile EFI variables to file >
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index 62e071bd83..fe1c462f17 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -192,8 +192,10 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe) ret = efi_var_mem_ins(var->name, &var->guid, var->attr, var->length, data, 0, NULL, var->time); - if (ret != EFI_SUCCESS) + if (ret != EFI_SUCCESS) { log_err("Failed to set EFI variable %ls\n", var->name); + return ret; + } } return EFI_SUCCESS; } diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index be95ed44e6..2b2ca8c090 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -350,8 +350,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, if (var_type == EFI_AUTH_VAR_PK) ret = efi_init_secure_state(); - else - ret = EFI_SUCCESS; /* * Write non-volatile EFI variables to file
Try to catch error the earlier way. Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> --- lib/efi_loader/efi_var_file.c | 4 +++- lib/efi_loader/efi_variable.c | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-)