Message ID | 786fd4842c59a6c4828533c8371089d07971938d.1532469861.git.geoff@infradead.org |
---|---|
State | Superseded |
Headers | show |
Series | [v1,01/30] docker: Add libfdt-dev | expand |
On Tue, 2018-07-24 at 22:15 +0000, Geoff Levand wrote: > From: Ge Song <ge.song@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@hxt-semitech.com> > [Cleanup file comments, make efivarfs_path static.] > Signed-off-by: Geoff Levand <geoff@infradead.org> > --- > lib/Makefile.am | 4 +- > lib/efi/efivar.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++ > lib/efi/efivar.h | 46 ++++++++++++ > test/lib/test-efivar.c | 127 ++++++++++++++++++++++++++++++++ > 4 files changed, 367 insertions(+), 1 deletion(-) > create mode 100644 lib/efi/efivar.c > create mode 100644 lib/efi/efivar.h > create mode 100644 test/lib/test-efivar.c Hi Geoff, The new efivar test isn't added to test/lib/Makefile so it doesn't run. I added it in but it looks like it might have gone a bit stale, eg: ../configure --enable-platform-powerpc --enable-platform-arm64 ... make && make check ... CC test/lib/test-efivar.o ../test/lib/test-efivar.c: In function ‘main’: ../test/lib/test-efivar.c:100:47: error: passing argument 3 of ‘efi_set_variable’ from incompatible pointer type [-Werror=incompatible-pointer-types] rc = efi_set_variable(ctx, test_efivar_guid, test_varname, ^~~~~~~~~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:50:26: note: expected ‘const struct efi_data *’ but argument is of type ‘const char *’ const struct efi_data *efi_data); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../test/lib/test-efivar.c:100:7: error: too many arguments to function ‘efi_set_variable’ rc = efi_set_variable(ctx, test_efivar_guid, test_varname, ^~~~~~~~~~~~~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:49:5: note: declared here int efi_set_variable(const char *guidstr, const char *name, ^~~~~~~~~~~~~~~~ ../test/lib/test-efivar.c:104:5: error: passing argument 4 of ‘efi_get_variable’ from incompatible pointer type [-Werror=incompatible-pointer-types] &data, &size, &attr); ^~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:48:21: note: expected ‘struct efi_data **’ but argument is of type ‘uint8_t **’ {aka ‘unsigned char **’} struct efi_data **efi_data); ~~~~~~~~~~~~~~~~~~^~~~~~~~ ../test/lib/test-efivar.c:103:7: error: too many arguments to function ‘efi_get_variable’ rc = efi_get_variable(ctx, test_efivar_guid, test_varname, ^~~~~~~~~~~~~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:47:5: note: declared here int efi_get_variable(void *ctx, const char *guidstr, const char *name, ^~~~~~~~~~~~~~~~ ../test/lib/test-efivar.c:113:7: error: too many arguments to function ‘efi_del_variable’ rc = efi_del_variable(ctx, test_efivar_guid, test_varname); ^~~~~~~~~~~~~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:51:5: note: declared here int efi_del_variable(const char *guidstr, const char *name); ^~~~~~~~~~~~~~~~ ../test/lib/test-efivar.c:116:5: error: passing argument 4 of ‘efi_get_variable’ from incompatible pointer type [-Werror=incompatible-pointer-types] &data, &size, &attr); ^~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:48:21: note: expected ‘struct efi_data **’ but argument is of type ‘uint8_t **’ {aka ‘unsigned char **’} struct efi_data **efi_data); ~~~~~~~~~~~~~~~~~~^~~~~~~~ ../test/lib/test-efivar.c:115:7: error: too many arguments to function ‘efi_get_variable’ rc = efi_get_variable(ctx, test_efivar_guid, test_varname, ^~~~~~~~~~~~~~~~ In file included from ../test/lib/test-efivar.c:32: ../lib/efi/efivar.h:47:5: note: declared here int efi_get_variable(void *ctx, const char *guidstr, const char *name, ^~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[3]: *** [Makefile:3930: test/lib/test-efivar.o] Error 1 make[3]: Leaving directory '/home/sam/source/petitboot/out' make[2]: *** [Makefile:6215: check-am] Error 2 make[2]: Leaving directory '/home/sam/source/petitboot/out' make[1]: *** [Makefile:5134: check-recursive] Error 1 make[1]: Leaving directory '/home/sam/source/petitboot/out' make: *** [Makefile:6219: check] Error 2
On Wed, 2018-08-01 at 16:54 +1000, Samuel Mendoza-Jonas wrote: > On Tue, 2018-07-24 at 22:15 +0000, Geoff Levand wrote: > > From: Ge Song <ge.song@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@hxt-semitech.com> > > [Cleanup file comments, make efivarfs_path static.] > > Signed-off-by: Geoff Levand <geoff@infradead.org> > > --- > > lib/Makefile.am | 4 +- > > lib/efi/efivar.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++ > > lib/efi/efivar.h | 46 ++++++++++++ > > test/lib/test-efivar.c | 127 ++++++++++++++++++++++++++++++++ > > 4 files changed, 367 insertions(+), 1 deletion(-) > > create mode 100644 lib/efi/efivar.c > > create mode 100644 lib/efi/efivar.h > > create mode 100644 test/lib/test-efivar.c > > Hi Geoff, > > The new efivar test isn't added to test/lib/Makefile so it doesn't run. > I added it in but it looks like it might have gone a bit stale, eg: > <snip> Looks like just the use of set/get are stale; that and a small fixup to efi_get_variable() makes everything green. Does this look sane? --- lib/efi/efivar.c | 3 ++- test/lib/Makefile.am | 3 ++- test/lib/test-efivar.c | 25 +++++++++++++++---------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/efi/efivar.c b/lib/efi/efivar.c index 0c4b462..4304562 100644 --- a/lib/efi/efivar.c +++ b/lib/efi/efivar.c @@ -148,7 +148,8 @@ int efi_get_variable(void *ctx, const char *guidstr, const char *name, rc = 0; exit: talloc_free(path); - close(fd); + if (fd >= 0) + close(fd); return rc; } diff --git a/test/lib/Makefile.am b/test/lib/Makefile.am index 047fcb2..65991a5 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 if WITH_OPENSSL lib_TESTS += \ diff --git a/test/lib/test-efivar.c b/test/lib/test-efivar.c index 8ceb8f5..a85b73c 100644 --- a/test/lib/test-efivar.c +++ b/test/lib/test-efivar.c @@ -87,33 +87,38 @@ int main(void) { void *ctx = NULL; int rc, errno_value; - size_t size; - uint8_t *data = NULL; uint32_t attr = DEF_ATTR; char *path = NULL; + struct efi_data *efi_data; if(!probe()) return ENOENT; talloc_new(ctx); - size = strlen(test_data) + 1; - rc = efi_set_variable(ctx, test_efivar_guid, test_varname, - (uint8_t *)test_data, size, attr); + efi_data = talloc_zero(ctx, struct efi_data); + efi_data->attributes = attr; + efi_data->data = talloc_strdup(efi_data, test_data); + efi_data->data_size = strlen(test_data) + 1; + + rc = efi_set_variable(test_efivar_guid, test_varname, + efi_data); + + talloc_free(efi_data); rc = efi_get_variable(ctx, test_efivar_guid, test_varname, - &data, &size, &attr); + &efi_data); - assert(data != NULL); - rc = strcmp((char *)data, test_data); + assert(efi_data->data != NULL); + rc = strcmp((char *)efi_data->data, test_data); if (rc) { talloc_free(ctx); assert(0); } - rc = efi_del_variable(ctx, test_efivar_guid, test_varname); + rc = efi_del_variable(test_efivar_guid, test_varname); rc = efi_get_variable(ctx, test_efivar_guid, test_varname, - &data, &size, &attr); + &efi_data); errno_value = errno; talloc_free(ctx);
Hi Sam, On 08/01/2018 10:43 PM, Samuel Mendoza-Jonas wrote: > On Wed, 2018-08-01 at 16:54 +1000, Samuel Mendoza-Jonas wrote: >> On Tue, 2018-07-24 at 22:15 +0000, Geoff Levand wrote: >> The new efivar test isn't added to test/lib/Makefile so it doesn't run. >> I added it in but it looks like it might have gone a bit stale, eg: > > Looks like just the use of set/get are stale; that and a small fixup to > efi_get_variable() makes everything green. Does this look sane? > @@ -148,7 +148,8 @@ int efi_get_variable(void *ctx, const char *guidstr, const char *name, > rc = 0; > exit: > talloc_free(path); > - close(fd); > + if (fd >= 0) > + close(fd); For consistency with the other routines, I think a return if efi_open() fails would be better. > return rc; > } I made the small change above and folded your changes into my V2 patches. Thanks for looking into it. -Geoff
diff --git a/lib/Makefile.am b/lib/Makefile.am index 0088e0b..59d37ab 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -65,7 +65,9 @@ lib_libpbcore_la_SOURCES = \ lib/util/util.h \ lib/flash/config.h \ lib/flash/flash.h \ - lib/security/security.h + lib/security/security.h \ + lib/efi/efivar.h \ + lib/efi/efivar.c if ENABLE_MTD lib_libpbcore_la_SOURCES += \ diff --git a/lib/efi/efivar.c b/lib/efi/efivar.c new file mode 100644 index 0000000..1ac6990 --- /dev/null +++ b/lib/efi/efivar.c @@ -0,0 +1,191 @@ +/* + * 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@hxt-semitech.com> + */ + +#include <errno.h> +#include <fcntl.h> +#include <stdlib.h> +#include <string.h> + +#include <linux/fs.h> +#include <sys/ioctl.h> + +#include "efivar.h" +#include "log/log.h" +#include "talloc/talloc.h" + +static const char *efivarfs_path; + +inline void set_efivarfs_path(const char *path) +{ + efivarfs_path = path; +} + +inline const char *get_efivarfs_path(void) +{ + + return efivarfs_path; +} + +int efi_del_variable(void *ctx, const char *guidstr, + const char *name) +{ + int fd, flag, errno_value; + int rc = -1; + const char *dir; + char *path; + + dir = get_efivarfs_path(); + if (!dir) + return -1; + + path = talloc_asprintf(ctx, "%s%s-%s", dir, name, guidstr); + if (!path) + return -1; + + 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; + const char *dir; + char *path; + + dir = get_efivarfs_path(); + if (!dir) + return EFAULT; + + path = talloc_asprintf(ctx, "%s%s-%s", dir, name, guidstr); + if (!path) + return ENOMEM; + + fd = open(path, O_RDONLY|O_NONBLOCK); + 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; + const char *dir; + char *path; + void *buf; + size_t bufsize; + mode_t mask = 0644; + + dir = get_efivarfs_path(); + if (!dir) + return EFAULT; + + path = talloc_asprintf(ctx, "%s%s-%s", dir, 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/lib/efi/efivar.h b/lib/efi/efivar.h new file mode 100644 index 0000000..ebf73fa --- /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@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 + +void set_efivarfs_path(const char *path); +const char *get_efivarfs_path(void); + +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/test/lib/test-efivar.c b/test/lib/test-efivar.c new file mode 100644 index 0000000..8ceb8f5 --- /dev/null +++ b/test/lib/test-efivar.c @@ -0,0 +1,127 @@ +/* + * 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@hxt-semitech.com> + */ +#include <assert.h> +#include <dirent.h> +#include <errno.h> +#include <fcntl.h> +#include <linux/limits.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/statfs.h> +#include <unistd.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 char* find_efitest_path(void) +{ + static char dir[PATH_MAX] = {0}; + static bool run = false; + char *rest_path = "/efivarfs_data/"; + char *pos = NULL; + + if (run) + return dir; + + readlink("/proc/self/exe", dir, PATH_MAX); + + pos = strrchr(dir, '/'); + *pos = '\0'; + + strcat(dir, rest_path); + run = true; + + return dir; +} + +static bool probe(void) +{ + char *path; + int rc; + + path = find_efitest_path(); + + rc = access(path, F_OK); + if (rc) { + if (errno == ENOENT) { + rc = mkdir(path, 0755); + if(rc) + return false; + } else { + return false; + } + } + + set_efivarfs_path(path); + + return true; +} + +int main(void) +{ + void *ctx = NULL; + int rc, errno_value; + size_t size; + uint8_t *data = NULL; + uint32_t attr = DEF_ATTR; + char *path = NULL; + + if(!probe()) + return ENOENT; + + 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); + + assert(data != NULL); + rc = strcmp((char *)data, test_data); + 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); + + path = find_efitest_path(); + rmdir(path); + + return EXIT_SUCCESS; +}