[Patch v3 1/2] lib: Support to deal with petitboot's configuration on efi-based platforms
Ge Song
songgebird at gmail.com
Sat May 5 00:29:56 AEST 2018
On 05/04/2018 09:32 AM, Samuel Mendoza-Jonas wrote:
> 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 :)
Hi Sam, sorry for the inconvenience. I have got little experience in
submitting code to
open source community. Thanks for your instructions:)
>> 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 :)
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 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?
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:)
> 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;
>> +}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/petitboot/attachments/20180504/cda711bc/attachment-0001.html>
More information about the Petitboot
mailing list