<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>On 04/30/2018 12:29 PM, Samuel Mendoza-Jonas wrote:<br>
</p>
<blockquote type="cite"
cite="mid:e38046ef0ed3560d0ea856efcc7a71ba8ccc7021.camel@mendozajonas.com">
<pre wrap="">On Mon, 2018-04-09 at 10:22 +0800, Ge Song wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Provide methods to load/store petitboot's configuration on efi-based
platforms. A test case is also provided.
Signed-off-by: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
---
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 <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+#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 <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+
+#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));
</pre>
</blockquote>
<pre wrap="">
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?</pre>
</blockquote>
<font face="Courier New, Courier, monospace"><font face="Courier 10
Pitch"><font face="Courier 10 Pitch"><font face="Times New
Roman, Times, serif">Yes, you are right. In
platform-powerpc.c, value of each parameter is stored in the
form of<br>
string(even mac address, http proxy ...). To be consistent
with it and reuse the routings,<br>
the same manner is used.<br>
<br>
Regard to efi variable, four elements are cared: attribute,
guid(can think of this as namespace), data and<br>
data size. Since value of each parameter is stored as a
string(null-terminated), when we set it,<br>
we set the data size as "strlen(string) + 1" to make '\0' is
also stored in efi var. When we read it,<br>
efivarfs will return the string(with '\0' in the end) in the
buffer.<br>
<br>
The size returned from stat() seems incorrect, since the
inode->getattr() is not implemented in efivarfs.<br>
</font></font></font></font>
<blockquote type="cite"
cite="mid:e38046ef0ed3560d0ea856efcc7a71ba8ccc7021.camel@mendozajonas.com">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ 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 <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+#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[])
+{
</pre>
</blockquote>
<pre wrap="">
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():
</pre>
</blockquote>
<font face="Times New Roman, Times, serif">Sorry, the prototype of
main() will be correct in next patch.<br>
Initially the test case was designed to get the parameters from
command line, after<br>
some thought it is not a good solution since one has to spend time
to find out<br>
the right parameters to be passed. </font><br>
<blockquote type="cite"
cite="mid:e38046ef0ed3560d0ea856efcc7a71ba8ccc7021.camel@mendozajonas.com">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ 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);
</pre>
</blockquote>
<pre wrap="">
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</pre>
</blockquote>
<font face="Times New Roman, Times, serif">Sorry for this, I made a
mistake, a probe()(as probe() in platform-xxx.c) is needed to
judge<br>
if efivarfs is already mounted prior to all the operations. In
fact I debugged it in gdb too,<br>
because efivarfs is mounted in my system automatically, it failed
to detect the issue.<br>
<br>
BTW, I'm not familiar with the test rules. If probe() failed,
which return code is reasonable? Or<br>
EXIT_SUCCESS is enough?<br>
</font>
<blockquote type="cite"
cite="mid:e38046ef0ed3560d0ea856efcc7a71ba8ccc7021.camel@mendozajonas.com">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ 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;
+}
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>