[Patch v3 1/2] lib: Support to deal with petitboot's configuration on efi-based platforms

[Samuel Mendoza-Jonas]

On Fri, 2018-05-04 at 22:29 +0800, Ge Song wrote:
<snip>

> > > ++int main(void)+{+	void *ctx = NULL;+	int rc, errno_value;+	size_t size;+	uint8_t *data = NULL;+	uint32_t attr = DEF_ATTR;++	if(!probe())+		return EXIT_SUCCESS;++	talloc_new(ctx);+	size = strlen(test_data) + 1;+	rc = efi_set_variable(ctx, test_efivar_guid, test_varname,+				(uint8_t *)test_data, size, attr);++	rc = efi_get_variable(ctx, test_efivar_guid, test_varname,+				&data, &size, &attr);++	rc = strcmp((char *)data, test_data);
> > > 
> > >       
> > 
> >       This still segfaults:
> > 	(gdb) frame 1	#1  0x0000555555554f94 in main () at ../test/lib/test-efivar.c:80	80		rc = strcmp((char *)data, test_data);
> > Most likely because efi_get_variable() is returning an error, likely sinceefi_set_variable() is failing because /sys/firmware/efi/efivars is onlywritable by root on my machine.I could just run the test as root and it would probably pass, but I don't wantto be writing random efi variables to my (or anyone elses) laptops as part of aPetitboot test :)
> > 
> >     
> 
>     Yes, operations under that
>       folder require root permission. I forgot to mention that since
>       usually
> 
>       petitboot will be executed with the root permission , so sorry for
>       it.
> 
>       
> 
>       In fact, the temporary variable set in NVRAM will be deleted in
>       the end of the function, when
> 
>       it returns normally, the variable won't be found in efivarfs
>       anymore.
> 
>       
> 
>       To be precise, the delete operation causes one bit in variable's
>       attribute field(in NVRAM) transition
> 
>       from 0(valid) to 1(invalid), when this happens, both firmware and
>       OS will ignore this variable and
> 
>       no side effect will be introduced. 
> 
>       
> 
>       The shortcoming is a little space is occupied by this temporary
>       variable, although it has been deleted.
> 
>       Don't worry about it, when the variable storage is full, a reclaim
>       action will be taken by firmware.
> 
>     
>     
> >       What would be better would be to have a 'fake' efivar directory that we set upas part of the test, and write and read changes to that. Then we're justtesting that our writing and parsing of the efivarfs format works, not testingthe efivarfs implementation of whatever machine we're on.
> > For example we could have a test/lib/efivarfs directory with some dummy filesin there in the efivarfs format. Then if we change the efivarfs path Petitbootuses the test can write to that without affecting the system. What do you think?
> > 
> >     
> 
>     Sounds like writing a
>       simple userspace filesystem? I agree with you, that's really a
>       good solution,
> 
>       and it's a interesting work, maybe more time is needed, I have to
>       study how to accomplish it first:) 

That would be interesting but I won't ask you to put in that much work for this series! An easier approach would just be
to have get_efivarfs_path() return a path to the test directory, eg. "./test/lib/efivarfs_data/" and within that write a few
files in the efivarfs format. We don't need to write to the real efivarfs for a test since we're just testing our parsing of it.
Besides a lot of testing runs on machines without access to efivarfs (on a Jenkins box for instance), so we don't want to
have to guarantee that it exists.

Have a look at test/parser/data/ for example - it's just a collection of "example" files we can test ourselves against, I
imagine we could do something very similar for lib/efi as well.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/petitboot/attachments/20180511/ea1288d9/attachment.html>