diff mbox series

[v1,15/30] lib/efi: Add new routines to access efi variables

Message ID 786fd4842c59a6c4828533c8371089d07971938d.1532469861.git.geoff@infradead.org
State Superseded
Headers show
Series [v1,01/30] docker: Add libfdt-dev | expand

Commit Message

Geoff Levand July 24, 2018, 10:15 p.m. UTC
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

Comments

Sam Mendoza-Jonas Aug. 1, 2018, 6:54 a.m. UTC | #1
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
Sam Mendoza-Jonas Aug. 2, 2018, 5:43 a.m. UTC | #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);
Geoff Levand Aug. 2, 2018, 5:02 p.m. UTC | #3
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 mbox series

Patch

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