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

Ge Song songgebird at gmail.com
Tue May 1 19:07:29 AEST 2018


On 04/30/2018 12:29 PM, Samuel Mendoza-Jonas wrote:

> On Mon, 2018-04-09 at 10:22 +0800, Ge Song wrote:
>> Provide methods to load/store petitboot's configuration on efi-based
>> platforms. A test case is also provided.
>>
>> Signed-off-by: Ge Song <ge.song at hxt-semitech.com>
>> ---
>>   lib/Makefile.am        |   2 +
>>   test/lib/Makefile.am   |   3 +-
>>   lib/efi/efivar.h       |  43 +++++
>>   lib/efi/efivar.c       | 179 ++++++++++++++++++++
>>   test/lib/test-efivar.c |  68 ++++++++
>>   5 files changed, 294 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>> index bb7dfe489132..50fb9d14df49 100644
>> --- a/lib/Makefile.am
>> +++ b/lib/Makefile.am
>> @@ -58,6 +58,8 @@ lib_libpbcore_la_SOURCES = \
>>   	lib/util/util.h \
>>   	lib/flash/config.h \
>>   	lib/flash/flash.h \
>> +	lib/efi/efivar.h \
>> +	lib/efi/efivar.c \
>>   	$(gpg_int_SOURCES)
>>   
>>   if ENABLE_MTD
>> diff --git a/test/lib/Makefile.am b/test/lib/Makefile.am
>> index 9636b08d6a6b..22e3ac91499d 100644
>> --- a/test/lib/Makefile.am
>> +++ b/test/lib/Makefile.am
>> @@ -23,7 +23,8 @@ lib_TESTS = \
>>   	test/lib/test-process-parent-stdout \
>>   	test/lib/test-process-both \
>>   	test/lib/test-process-stdout-eintr \
>> -	test/lib/test-fold
>> +	test/lib/test-fold \
>> +	test/lib/test-efivar
>>   
>>   $(lib_TESTS): LIBS += $(core_lib)
>>   
>> diff --git a/lib/efi/efivar.h b/lib/efi/efivar.h
>> new file mode 100644
>> index 000000000000..da0e0d68b6c8
>> --- /dev/null
>> +++ b/lib/efi/efivar.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> + *
>> + *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
>> + *  reserved.
>> + *  Author: Ge Song <ge.song at hxt-semitech.com>
>> + */
>> +#ifndef EFIVAR_H
>> +#define EFIVAR_H
>> +
>> +#include <stdint.h>
>> +
>> +#define EFI_VARIABLE_NON_VOLATILE                           0x00000001
>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS                     0x00000002
>> +#define EFI_VARIABLE_RUNTIME_ACCESS                         0x00000004
>> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD                  0x00000008
>> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS             0x00000010
>> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS  0x00000020
>> +#define EFI_VARIABLE_APPEND_WRITE                           0x00000040
>> +
>> +#define EFIVARFS_MAGIC 0xde5e81e4
>> +
>> +static inline const char* get_efivarfs_path(void)
>> +{
>> +	return "/sys/firmware/efi/efivars/";
>> +}
>> +int efi_get_variable(void *ctx, const char *guidstr, const char *name,
>> +		uint8_t **data, size_t *data_size, uint32_t *attributes);
>> +int efi_set_variable(void *ctx, const char *guidstr, const char *name,
>> +		uint8_t *data, size_t data_size, uint32_t attributes);
>> +int efi_del_variable(void *ctx, const char *guidstr, const char *name);
>> +#endif /* EFIVAR_H */
>> diff --git a/lib/efi/efivar.c b/lib/efi/efivar.c
>> new file mode 100644
>> index 000000000000..5ce3464b9e77
>> --- /dev/null
>> +++ b/lib/efi/efivar.c
>> @@ -0,0 +1,179 @@
>> +/*
>> + *  Methods to manipulation of EFI variables on UEFI-based platforms
>> + *
>> + *  The library relies on pesudo file system named "efivarfs". This is the
>> + *  new interface to manipulate EFI varibles and was part of the linux
>> + *  kernel v3.10 release.
>> + *
>> + *  There is also a legacy efivars interface exported via the path
>> + *  "firmware/efi/vars/" relative to the mount point of sysfs. Since it has
>> + *  some drawbacks and is deprecated, we don't support this interface.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> + *
>> + *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
>> + *  reserved.
>> + *  Author: Ge Song <ge.song at hxt-semitech.com>
>> + */
>> +
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <linux/fs.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +
>> +#include "efivar.h"
>> +#include "talloc/talloc.h"
>> +
>> +int efi_del_variable(void *ctx, const char *guidstr,
>> +		const char *name)
>> +{
>> +	int fd, flag, errno_value;
>> +	int rc = -1;
>> +	char *path;
>> +
>> +	path = talloc_asprintf(ctx, "%s%s-%s",
>> +				get_efivarfs_path(), name, guidstr);
>> +	if (!path)
>> +		return ENOMEM;
>> +
>> +	fd = open(path, O_RDONLY|O_NONBLOCK);
>> +	if (fd == -1)
>> +		goto err;
>> +
>> +	rc = ioctl(fd, FS_IOC_GETFLAGS, &flag);
>> +	if (rc == -1)
>> +		goto err;
>> +
>> +	flag &= ~FS_IMMUTABLE_FL;
>> +	rc = ioctl(fd, FS_IOC_SETFLAGS, &flag);
>> +	if (rc == -1)
>> +		goto err;
>> +
>> +	close(fd);
>> +	rc = unlink(path);
>> +
>> +err:
>> +	errno_value = errno;
>> +	if (fd > 0)
>> +		close(fd);
>> +
>> +	errno = errno_value;
>> +	return rc;
>> +}
>> +
>> +int efi_get_variable(void *ctx, const char *guidstr, const char *name,
>> +		uint8_t **data, size_t *data_size, uint32_t *attributes)
>> +{
>> +	int fd, errno_value;
>> +	int rc = -1;
>> +	void *p, *buf;
>> +	size_t bufsize = 4096;
>> +	size_t filesize = 0;
>> +	ssize_t sz;
>> +	char *path;
>> +
>> +	path = talloc_asprintf(ctx, "%s%s-%s",
>> +				get_efivarfs_path(), name, guidstr);
>> +	if (!path)
>> +		return ENOMEM;
>> +
>> +	fd = open(path, O_RDONLY);
>> +	if (fd < 0)
>> +		goto err;
>> +
>> +	buf = talloc_size(ctx, bufsize);
>> +	if (!buf)
>> +		goto err;
>> +
>> +	do {
>> +		p = buf + filesize;
>> +		sz = read(fd, p, bufsize);
>> +		if (sz < 0 && errno == EAGAIN) {
>> +			continue;
>> +		} else if (sz == 0) {
>> +			break;
>> +		}
>> +		filesize += sz;
>> +	} while (1);
>> +
>> +	*attributes = *(uint32_t *)buf;
>> +	*data = (uint8_t *)(buf + sizeof(uint32_t));
>> +	*data_size = strlen(buf + sizeof(uint32_t));
> I'm a little unsure about this strlen() call. Why not just find the variable
> size with stat()? Here it looks like we're trying to read the file as a string
> but are we sure it will be null-terminated?
Yes, you are right. In platform-powerpc.c, value of each parameter is 
stored in the form of
string(even mac address, http proxy ...). To be consistent with it and 
reuse the routings,
the same manner is used.

Regard to efi variable, four elements are cared: attribute, guid(can 
think of this as namespace), data and
data size. Since value of each parameter is stored as a 
string(null-terminated), when we set it,
we set the data size as "strlen(string) + 1" to make '\0' is also stored 
in efi var. When we read it,
efivarfs will return the string(with '\0' in the end) in the buffer.

The size returned from stat() seems incorrect, since the 
inode->getattr() is not implemented in efivarfs.
>
>> +	rc = 0;
>> +
>> +err:
>> +	errno_value = errno;
>> +	if (fd > 0)
>> +		close(fd);
>> +
>> +	errno = errno_value;
>> +	return rc;
>> +}
>> +
>> +int efi_set_variable(void *ctx, const char *guidstr, const char *name,
>> +		uint8_t *data, size_t data_size, uint32_t attributes)
>> +{
>> +	int rc = -1, errno_value;
>> +	int fd = -1;
>> +	ssize_t len;
>> +	char *path;
>> +	void *buf;
>> +	size_t bufsize;
>> +	mode_t mask = umask(umask(0));
>> +
>> +	path = talloc_asprintf(ctx, "%s%s-%s",
>> +				get_efivarfs_path(), name, guidstr);
>> +	if (!path)
>> +		return ENOMEM;
>> +
>> +	if (!access(path, F_OK)) {
>> +		rc = efi_del_variable(ctx, guidstr, name);
>> +		if (rc < 0) {
>> +			goto err;
>> +		}
>> +	}
>> +
>> +	fd = open(path, O_CREAT|O_WRONLY, mask);
>> +	if (fd < 0)
>> +		goto err;
>> +
>> +	bufsize = sizeof(uint32_t) + data_size;
>> +	buf = talloc_size(ctx, bufsize);
>> +	if (!buf)
>> +		goto err;
>> +
>> +	*(uint32_t *)buf = attributes;
>> +	memcpy(buf + sizeof(uint32_t), data, data_size);
>> +
>> +	len = write(fd, buf, bufsize);
>> +	if ((size_t)len != bufsize)
>> +		goto err;
>> +	else
>> +		rc = 0;
>> +
>> +err:
>> +	errno_value = errno;
>> +	if (fd > 0)
>> +		close(fd);
>> +
>> +	errno = errno_value;
>> +	return rc;
>> +}
>> diff --git a/test/lib/test-efivar.c b/test/lib/test-efivar.c
>> new file mode 100644
>> index 000000000000..aec2549c4993
>> --- /dev/null
>> +++ b/test/lib/test-efivar.c
>> @@ -0,0 +1,68 @@
>> +/*
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> + *
>> + *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
>> + *  reserved.
>> + *  Author: Ge Song <ge.song at hxt-semitech.com>
>> + */
>> +#include <assert.h>
>> +#include <errno.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +
>> +#include "efi/efivar.h"
>> +#include "talloc/talloc.h"
>> +
>> +#define DEF_ATTR	(EFI_VARIABLE_NON_VOLATILE | \
>> +	EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
>> +
>> +static const char *test_efivar_guid = "c9c07add-256e-4452-b911-f8d0d35a1ac7";
>> +static const char *test_varname = "efivartest";
>> +static const char *test_data = "petitboot";
>> +
>> +int main(int argc, char *argv[])
>> +{
> This test case fails because argc and argv are unused:
>
> ../test/lib/test-efivar.c: In function ‘main’:
> ../test/lib/test-efivar.c:36:14: error: unused parameter ‘argc’ [-Werror=unused-parameter]
>   int main(int argc, char *argv[])
>                ^~~~
> ../test/lib/test-efivar.c:36:26: error: unused parameter ‘argv’ [-Werror=unused-parameter]
>   int main(int argc, char *argv[])
>                            ^~~~
>
> However after fixing that the test actually segfaults below on the
> strcmp():
Sorry, the prototype of main() will be correct in next patch.
Initially the test case was designed to get the parameters from command 
line, after
some thought it is not a good solution since one has to spend time to 
find out
the right parameters to be passed.
>> +	void *ctx = NULL;
>> +	int rc, errno_value;
>> +	size_t size;
>> +	uint8_t *data = NULL;
>> +	uint32_t attr = DEF_ATTR;
>> +
>> +	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);
> gdb ./test/lib/test-efivar
> ...
> (gdb) run
> Starting program: /home/sam/dev/petitboot/patches/out/test/lib/test-efivar
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7aaf67a in __strcmp_sse2_unaligned () from /usr/lib/libc.so.6
> (gdb) bt
> #0  0x00007ffff7aaf67a in __strcmp_sse2_unaligned () from /usr/lib/libc.so.6
> #1  0x0000555555554e2e in main () at ../test/lib/test-efivar.c:54
>
> (gdb) p data
> $1 = (uint8_t *) 0x0
Sorry for this, I made a mistake, a probe()(as probe() in 
platform-xxx.c) is needed to judge
if efivarfs is already mounted prior to all the operations. In fact I 
debugged it in gdb too,
because efivarfs is mounted in my system automatically, it failed to 
detect the issue.

BTW, I'm not familiar with the test rules. If probe() failed, which 
return code is reasonable? Or
EXIT_SUCCESS is enough?
>
>> +	if (rc) {
>> +		talloc_free(ctx);
>> +		assert(0);
>> +	}
>> +
>> +	rc = efi_del_variable(ctx, test_efivar_guid, test_varname);
>> +
>> +	rc = efi_get_variable(ctx, test_efivar_guid, test_varname,
>> +				&data, &size, &attr);
>> +
>> +	errno_value = errno;
>> +	talloc_free(ctx);
>> +	assert(errno_value == ENOENT);
>> +
>> +	return EXIT_SUCCESS;
>> +}

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/petitboot/attachments/20180501/8b0b16de/attachment-0001.html>


More information about the Petitboot mailing list