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

Samuel Mendoza-Jonas sam at mendozajonas.com
Fri May 4 11:32:19 AEST 2018


On Wed, 2018-05-02 at 18:35 +0800, Ge Song wrote:
> From: Ge Song <ge.song at hxt-semitech.com>
> 
> 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>
> ---

Hi Ge Song, thanks for the updates. In future revisions please add a line here
under the '---' describing what changed since the last version, it helps to
keep track of changes :)

>  lib/Makefile.am        |   2 +
>  test/lib/Makefile.am   |   3 +-
>  lib/efi/efivar.h       |  46 +++++
>  lib/efi/efivar.c       | 179 ++++++++++++++++++++
>  test/lib/test-efivar.c |  93 ++++++++++
>  5 files changed, 322 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..3dfec02d319d
> --- /dev/null
> +++ b/lib/efi/efivar.h
> @@ -0,0 +1,46 @@
> +/*
> + *  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 <linux/magic.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
> +
> +#ifndef EFIVARFS_MAGIC
> +#define EFIVARFS_MAGIC 0xde5e81e4
> +#endif
> +
> +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));
> +	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..b952ee6b3018
> --- /dev/null
> +++ b/test/lib/test-efivar.c
> @@ -0,0 +1,93 @@
> +/*
> + *  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 <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/statfs.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";
> +
> +static bool probe(void)
> +{
> +	const char *path = "/sys/firmware/efi/efivars/";
> +	int rc = 0;
> +	struct statfs buf;
> +
> +	if (access(path, F_OK))
> +		return false;
> +
> +	memset(&buf, '\0', sizeof(buf));
> +	rc = statfs(path, &buf);
> +	if (rc)
> +		return false;
> +
> +	typeof(buf.f_type) magic = EFIVARFS_MAGIC;
> +	if (!memcmp(&buf.f_type, &magic, sizeof (magic)))
> +		return true;
> +
> +	return false;
> +}
> +
> +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 since
efi_set_variable() is failing because /sys/firmware/efi/efivars is only
writable by root on my machine.
I could just run the test as root and it would probably pass, but I don't want
to be writing random efi variables to my (or anyone elses) laptops as part of a
Petitboot test :)

What would be better would be to have a 'fake' efivar directory that we set up
as part of the test, and write and read changes to that. Then we're just
testing that our writing and parsing of the efivarfs format works, not testing
the efivarfs implementation of whatever machine we're on.

For example we could have a test/lib/efivarfs directory with some dummy files
in there in the efivarfs format. Then if we change the efivarfs path Petitboot
uses the test can write to that without affecting the system. What do you think?

Cheers,
Sam

> +	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;
> +}



More information about the Petitboot mailing list