libefivar: get rid of our variadic mess in efi_set_variable()

Sometimes the linked output wasn't correctly choosing the right
functions, and we'd get undefined behavior and the result was random
file modes on efi variables.

Instead, require the mode, and provide a compatibility symbol with the
older version that will use a safe (if mildly annoying) mode of
(0600 & ~umask).

Signed-off-by: Peter Jones <pjones@redhat.com>

Make.rules

@@ -19,6 +19,7 @@ include $(TOPDIR)/Make.version
 %.so :
 	$(CCLD) $(ccldflags) $(CPPFLAGS) $(SOFLAGS) \
 	  -Wl,-soname,$@.$(MAJOR_VERSION) \
+	  -Wl,--version-script=$(MAP) \
 	  -o $@ $^ $(LDLIBS)
 
 %.o : %.c

gcc.specs

@@ -14,4 +14,4 @@
 + %{!shared:%{!static:%{!r:-pie}}} %{static:-Wl,-no-fatal-warnings -Wl,-static -static -Wl,-z,relro,-z,now}
 
 *link:
-+ %{!static:--fatal-warnings} --default-symver --no-undefined-version --no-allow-shlib-undefined --add-needed -z now --build-id %{!static:%{!shared:-PIE}} %{shared:-z relro -PIC} %{static:%<pie}
++ %{!static:--fatal-warnings} --no-undefined-version --no-allow-shlib-undefined --add-needed -z now --build-id %{!static:%{!shared:-PIE}} %{shared:-z relro -PIC} %{static:%<pie}

src/Makefile

@@ -45,8 +45,9 @@ libefivar.a : | $(GENERATED_SOURCES)
 libefivar.a : $(patsubst %.o,%.static.o,$(LIBEFIVAR_OBJECTS))
 
 libefivar.so : $(LIBEFIVAR_OBJECTS)
-libefivar.so : | $(GENERATED_SOURCES)
+libefivar.so : | $(GENERATED_SOURCES) libefivar.map
 libefivar.so : LIBS=dl
+libefivar.so : MAP=libefivar.map
 
 efivar : efivar.c | libefivar.so
 efivar : LIBS=efivar dl
@@ -60,6 +61,9 @@ efivar-static : PKGS=popt
 libefiboot.a : $(patsubst %.o,%.static.o,$(LIBEFIBOOT_OBJECTS))
 
 libefiboot.so : $(LIBEFIBOOT_OBJECTS)
+libefiboot.so : | libefiboot.map
+libefiboot.so : LIBS=efivar
+libefiboot.so : MAP=libefiboot.map
 
 deps :: $(ALL_SOURCES)
 	$(MAKE) -f $(SRCDIR)/Make.deps deps SOURCES="$(ALL_SOURCES)"

src/export.c

@@ -360,5 +360,5 @@ efi_variable_realize(efi_variable_t *var)
 					var->data, var->data_size, attrs);
 	}
 	return efi_set_variable(*var->guid, var->name, var->data,
-				var->data_size, attrs);
+				var->data_size, attrs, 0600);
 }

src/generics.h

@@ -144,15 +144,16 @@ generic_append_variable(efi_guid_t guid, const char *name,
 		 * really not much to do about it, so return the error and
 		 * let our caller attempt to clean up :/
 		 */
-		rc = _efi_set_variable(guid, name, d, ds, attributes, 0);
+		rc = efi_set_variable(guid, name, d, ds, attributes, 0600);
 		free(d);
 		free(data);
 		return rc;
 	} else if (errno == ENOENT) {
 		data = new_data;
 		data_size = new_data_size;
 		attributes = new_attributes & ~EFI_VARIABLE_APPEND_WRITE;
-		rc = _efi_set_variable(guid, name, data, data_size, attributes, 0);
+		rc = efi_set_variable(guid, name, data, data_size,
+				      attributes, 0600);
 		return rc;
 	}
 	return rc;

src/include/efivar/efivar.h

@@ -73,36 +73,10 @@ extern int efi_get_variable(efi_guid_t guid, const char *name, uint8_t **data,
 				__attribute__((__nonnull__ (2, 3, 4, 5)));
 extern int efi_del_variable(efi_guid_t guid, const char *name)
 				__attribute__((__nonnull__ (2)));
-extern int _efi_set_variable(efi_guid_t guid, const char *name,
-			     uint8_t *data, size_t data_size,
-			     uint32_t attributes, mode_t mode)
-			    __attribute__((__nonnull__ (2, 3)));
-extern int _efi_set_variable_variadic(efi_guid_t guid, const char *name,
-				      uint8_t *data, size_t data_size,
-				      uint32_t attributes, ...);
-				     __attribute__((__nonnull__ (2, 3)))
-extern inline int
-__attribute__((__gnu_inline__))
-__attribute__((__artificial__))
-__attribute__((__visibility__ ("default")))
-efi_set_variable(efi_guid_t guid, const char *name,
-			       uint8_t *data, size_t data_size,
-			       uint32_t attributes, ...)
-{
-	if (__builtin_va_arg_pack_len() != 0 &&
-			__builtin_va_arg_pack_len() != 1) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	if (__builtin_va_arg_pack_len() == 0)
-		return _efi_set_variable(guid, name, data, data_size,
-					 attributes, 0644);
-
-	return _efi_set_variable_variadic(guid, name, data, data_size,
-					  attributes, __builtin_va_arg_pack());
-}
-
+extern int efi_set_variable(efi_guid_t guid, const char *name,
+			    uint8_t *data, size_t data_size,
+			    uint32_t attributes, mode_t mode)
+				__attribute__((__nonnull__ (2, 3)));
 extern int efi_append_variable(efi_guid_t guid, const char *name,
 			       uint8_t *data, size_t data_size,
 			       uint32_t attributes)

src/lib.c

@@ -43,25 +43,31 @@ int
 __attribute__((__nonnull__ (2, 3)))
 __attribute__((__visibility__ ("default")))
 _efi_set_variable(efi_guid_t guid, const char *name, uint8_t *data,
-		 size_t data_size, uint32_t attributes, mode_t mode)
+		  size_t data_size, uint32_t attributes)
 {
-	return ops->set_variable(guid, name, data, data_size, attributes, mode);
+	return ops->set_variable(guid, name, data, data_size, attributes, 0600);
 }
+__asm__(".symver _efi_set_variable,_efi_set_variable@");
 
 int
 __attribute__((__nonnull__ (2, 3)))
 __attribute__((__visibility__ ("default")))
 _efi_set_variable_variadic(efi_guid_t guid, const char *name, uint8_t *data,
-                 size_t data_size, uint32_t attributes, ...)
+			   size_t data_size, uint32_t attributes, ...)
+{
+	return ops->set_variable(guid, name, data, data_size, attributes, 0600);
+}
+__asm__(".symver _efi_set_variable_variadic,efi_set_variable@");
+
+int
+__attribute__((__nonnull__ (2, 3)))
+__attribute__((__visibility__ ("default")))
+efi_set_variable(efi_guid_t guid, const char *name, uint8_t *data,
+		 size_t data_size, uint32_t attributes, mode_t mode)
 {
-	va_list ap;
-	va_start(ap, attributes);
-	mode_t mode = va_arg(ap, mode_t);
-	va_end(ap);
 	return ops->set_variable(guid, name, data, data_size, attributes, mode);
 }
-extern typeof(_efi_set_variable_variadic) efi_set_variable
-	__attribute__ ((alias ("_efi_set_variable_variadic")));
+__asm__(".symver efi_set_variable,efi_set_variable@@LIBEFIVAR_0.24");
 
 int
 __attribute__((__nonnull__ (2, 3)))

src/libefiboot.map

@@ -0,0 +1,26 @@
+libefiboot.so.0 {
+	global: efi_generate_file_device_path;
+		efi_generate_file_device_path_from_esp;
+		efi_generate_ipv4_device_path;
+		efi_va_generate_file_device_path_from_esp;
+		efi_loadopt_args_as_ucs2;
+		efi_loadopt_args_as_utf8;
+		efi_loadopt_args_from_file;
+		efi_loadopt_attr_clear;
+		efi_loadopt_attr_set;
+		efi_loadopt_attrs;
+		efi_loadopt_create;
+		efi_loadopt_desc;
+		efi_loadopt_is_valid;
+		efi_loadopt_optional_data;
+		efi_loadopt_optional_data_size;
+		efi_loadopt_path;
+		efi_loadopt_pathlen;
+	local: *;
+};
+
+LIBEFIBOOT_0.0 {
+} libefiboot.so.0;
+
+LIBEFIBOOT_0.24 {
+};

src/libefivar.map

@@ -0,0 +1,99 @@
+libefivar.so.0 {
+	global:	efi_append_variable;
+		efi_chmod_variable;
+		efi_del_variable;
+		efi_get_next_variable_name;
+		efi_get_variable;
+		efi_get_variable_attributes;
+		efi_get_variable_size;
+		efi_guid_is_empty;
+		efi_guid_to_id_guid;
+		efi_guid_to_name;
+		efi_guid_to_str;
+		efi_guid_to_symbol;
+		efi_id_guid_to_guid;
+		efi_name_to_guid;
+		_efi_set_variable;
+		_efi_set_variable_variadic;
+		efi_str_to_guid;
+		efi_variable_export;
+		efi_variable_free;
+		efi_variable_get_attributes;
+		efi_variable_get_data;
+		efi_variable_get_guid;
+		efi_variable_get_name;
+		efi_variable_import;
+		efi_variable_realize;
+		efi_variable_set_attributes;
+		efi_variable_set_data;
+		efi_variable_set_guid;
+		efi_variable_set_name;
+		efi_variables_supported;
+		efi_well_known_guids;
+		efi_well_known_guids_end;
+		efidp_append_instance;
+		efidp_append_node;
+		efidp_append_path;
+		efidp_duplicate_path;
+		efidp_format_device_path;
+		efidp_make_acpi_hid;
+		efidp_make_acpi_hid_ex;
+		efidp_make_atapi;
+		efidp_make_edd10;
+		efidp_make_file;
+		efidp_make_generic;
+		efidp_make_hd;
+		efidp_make_ipv4;
+		efidp_make_mac_addr;
+		efidp_make_nvme;
+		efidp_make_pci;
+		efidp_make_sas;
+		efidp_make_sata;
+		efidp_make_scsi;
+		efidp_make_vendor;
+		efidp_parse_device_node;
+		efidp_parse_device_path;
+		efidp_set_node_data;
+
+		efi_guid_empty;
+		efi_guid_global;
+		efi_guid_lenovo;
+		efi_guid_lenovo_2;
+		efi_guid_lenovo_boot_menu;
+		efi_guid_lenovo_diag;
+		efi_guid_lenovo_diag_splash;
+		efi_guid_lenovo_me_config;
+		efi_guid_lenovo_msg;
+		efi_guid_lenovo_rescue;
+		efi_guid_lenovo_setup;
+		efi_guid_lenovo_startup_interrupt;
+		efi_guid_microsoft;
+		efi_guid_pkcs7_cert;
+		efi_guid_redhat;
+		efi_guid_redhat_2;
+		efi_guid_rsa2048;
+		efi_guid_rsa2048_sha1;
+		efi_guid_rsa2048_sha256;
+		efi_guid_rsa2048_sha256_cert;
+		efi_guid_security;
+		efi_guid_sha1;
+		efi_guid_sha224;
+		efi_guid_sha256;
+		efi_guid_sha384;
+		efi_guid_sha512;
+		efi_guid_shell;
+		efi_guid_shim;
+		efi_guid_x509_cert;
+		efi_guid_x509_sha256;
+		efi_guid_x509_sha384;
+		efi_guid_x509_sha512;
+		efi_guid_zero;
+	local:	*;
+};
+
+LIBEFIVAR_0.0 {
+} libefivar.so.0;
+
+LIBEFIVAR_0.24 {
+	global: efi_set_variable;
+} LIBEFIVAR_0.0;

src/test/tester.c

@@ -106,7 +106,7 @@ int do_test(struct test *test)
 			      testdata, test->size,
 			      EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			      EFI_VARIABLE_RUNTIME_ACCESS |
-			      EFI_VARIABLE_NON_VOLATILE);
+			      EFI_VARIABLE_NON_VOLATILE, 0600);
 	if (rc < 0) {
 		report_error(test, ret, rc, "set test failed: %m\n");
 	}
@@ -159,21 +159,12 @@ int do_test(struct test *test)
 	if (rc < 0)
 		report_error(test, ret, rc, "del test failed: %m\n");
 
-	printf("testing efi_set_variable() with too many arguments\n");
 	rc = efi_set_variable(TEST_GUID, test->name,
 			      testdata, test->size,
 			      EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			      EFI_VARIABLE_RUNTIME_ACCESS |
-			      EFI_VARIABLE_NON_VOLATILE, 0644, 1);
-	if (rc < 0) {
-		report_error(test, ret, -1, "set test failed: %m\n");
-	}
-
-	rc = efi_set_variable(TEST_GUID, test->name,
-			      testdata, test->size,
-			      EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			      EFI_VARIABLE_RUNTIME_ACCESS |
-			      EFI_VARIABLE_NON_VOLATILE);
+			      EFI_VARIABLE_NON_VOLATILE,
+			      0600);
 	if (rc < 0) {
 		report_error(test, ret, rc, "set test failed: %m\n");
 	}