Hi Mark,
On Tue, 11 Feb 2025 at 10:17, Mark Kettenis <mark.kette...@xs4all.nl> wrote:
>
> > From: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> >
> > On EFI runtime services, we manage to preserve string literals
> > by placing the .efi_runtime section just before .data and preserving
> > it when fixing up the runtime memory by marking surrounding boottime
> > code as runtime. This is ok for now but will break if we update any
> > linker scripts and decouple .text and .runtime sections.
> >
> > So let's define the strings we used to compare in the stack for
> > runtime services
>
> I don't see how this helps you. Those string literals still need to
> be copied into the stack variable from somewhere and I don't think
> that with this changes where that source string literal lives...
>
> I think you need toput the string in a global variable with an
> appropriate __attribute__ that puts it into .runtime.
The asm I am seeing is setting this up on the stack
c29c0: d2800ac1 mov x1, #0x56 // #86
{
c29c4: a9ba7bfd stp x29, x30, [sp, #-96]!
const u16 t[] = u"VarToFile";
c29c8: f2a00c21 movk x1, #0x61, lsl #16
c29cc: f2c00e41 movk x1, #0x72, lsl #32
{
c29d0: 910003fd mov x29, sp
const u16 t[] = u"VarToFile";
c29d4: f2e00a81 movk x1, #0x54, lsl #48
c29d8: f90027e1 str x1, [sp, #72]
c29dc: d2800de1 mov x1, #0x6f // #111
{
c29e0: a90153f3 stp x19, x20, [sp, #16]
const u16 t[] = u"VarToFile";
c29e4: f2a008c1 movk x1, #0x46, lsl #16
c29e8: f2c00d21 movk x1, #0x69, lsl #32
{
c29ec: a9025bf5 stp x21, x22, [sp, #32]
const u16 t[] = u"VarToFile";
c29f0: f2e00d81 movk x1, #0x6c, lsl #48
{
c29f4: f9001bf7 str x23, [sp, #48]
const u16 t[] = u"VarToFile";
c29f8: f9002be1 str x1, [sp, #80]
c29fc: 52800ca1 mov w1, #0x65 // #101
c2a00: b9005be1 str w1, [sp, #88]
if (!variable_name || !vendor || !data_size)
While the older code is doing
if (!u16_strcmp(variable_name, u"VarToFile"))
a88: 90000001 adrp x1, 0 <__image_copy_start>
But I think that's very arm and compiler-specific, so I'll change it
to what you and Heinrich suggested
Thanks
/Ilias
>
> > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > ---
> > lib/efi_loader/efi_var_mem.c | 3 ++-
> > lib/efi_loader/efi_variable_tee.c | 3 ++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> > index b265d95dd6ba..985e0baa128d 100644
> > --- a/lib/efi_loader/efi_var_mem.c
> > +++ b/lib/efi_loader/efi_var_mem.c
> > @@ -310,6 +310,7 @@ efi_get_variable_mem(const u16 *variable_name, const
> > efi_guid_t *vendor,
> > {
> > efi_uintn_t old_size;
> > struct efi_var_entry *var;
> > + u16 vtf[] = u"VarToFile";
> > u16 *pdata;
> >
> > if (!variable_name || !vendor || !data_size)
> > @@ -331,7 +332,7 @@ efi_get_variable_mem(const u16 *variable_name, const
> > efi_guid_t *vendor,
> > if (timep)
> > *timep = var->time;
> >
> > - if (!u16_strcmp(variable_name, u"VarToFile"))
> > + if (!u16_strcmp(variable_name, vtf))
> > return efi_var_collect_mem(data, data_size,
> > EFI_VARIABLE_NON_VOLATILE);
> >
> > old_size = *data_size;
> > diff --git a/lib/efi_loader/efi_variable_tee.c
> > b/lib/efi_loader/efi_variable_tee.c
> > index 0d090d051dd4..8d173e58d2f7 100644
> > --- a/lib/efi_loader/efi_variable_tee.c
> > +++ b/lib/efi_loader/efi_variable_tee.c
> > @@ -780,6 +780,7 @@ efi_status_t efi_set_variable_int(const u16
> > *variable_name,
> > efi_uintn_t payload_size;
> > efi_uintn_t name_size;
> > u8 *comm_buf = NULL;
> > + u16 pk[] = u"PK";
> > bool ro;
> >
> > if (!variable_name || variable_name[0] == 0 || !vendor) {
> > @@ -858,7 +859,7 @@ efi_status_t efi_set_variable_int(const u16
> > *variable_name,
> > if (alt_ret != EFI_SUCCESS)
> > goto out;
> >
> > - if (!u16_strcmp(variable_name, u"PK"))
> > + if (!u16_strcmp(variable_name, pk))
> > alt_ret = efi_init_secure_state();
> > out:
> > free(comm_buf);
> > --
> > 2.43.0
> >
> >
