[PATCH 1/2] uefi: uefirtvariable: Add invalid NULL parameter sanity checks

Colin King colin.king at canonical.com
Thu Apr 2 15:59:59 UTC 2015 

From: Colin Ian King <colin.king at canonical.com>

UEFI allows NULL parameters to be passed in the GetVariable
run time service, so add NULL parameter checks to this test.

We need to modify the uefi driver to handle NULL parameters too.

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 efi_runtime/efi_runtime.c                |  59 +++++++++------
 src/uefi/uefirtvariable/uefirtvariable.c | 123 +++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 23 deletions(-)

diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
index 31e5bb3..86cda1a 100644
--- a/efi_runtime/efi_runtime.c
+++ b/efi_runtime/efi_runtime.c
@@ -237,13 +237,12 @@ static long efi_runtime_get_variable(unsigned long arg)
 {
 	struct efi_getvariable __user *pgetvariable;
 	struct efi_getvariable pgetvariable_local;
-	unsigned long datasize, prev_datasize;
-	EFI_GUID vendor_guid;
-	efi_guid_t vendor;
+	unsigned long datasize, prev_datasize, *pdatasize;
+	efi_guid_t vendor, *pvendor = NULL;
 	efi_status_t status;
-	uint16_t *name;
-	uint32_t attr;
-	void *data;
+	uint16_t *name = NULL;
+	uint32_t attr, *pattr;
+	void *data = NULL;
 	int rv = 0;
 
 	pgetvariable = (struct efi_getvariable __user *)arg;
@@ -251,31 +250,44 @@ static long efi_runtime_get_variable(unsigned long arg)
 	if (copy_from_user(&pgetvariable_local, pgetvariable,
 			   sizeof(pgetvariable_local)))
 		return -EFAULT;
+	if (get_user(datasize, pgetvariable_local.DataSize))
+		return -EFAULT;
+	if (pgetvariable_local.VendorGuid) {
+		EFI_GUID vendor_guid;
 
-	if (get_user(datasize, pgetvariable_local.DataSize) ||
-	    copy_from_user(&vendor_guid, pgetvariable_local.VendorGuid,
+		if (copy_from_user(&vendor_guid, pgetvariable_local.VendorGuid,
 			   sizeof(vendor_guid)))
-		return -EFAULT;
+			return -EFAULT;
+		convert_from_guid(&vendor, &vendor_guid);
+		pvendor = &vendor;
+	}
 
-	convert_from_guid(&vendor, &vendor_guid);
+	if (pgetvariable_local.VariableName) {
+		rv = copy_ucs2_from_user(&name, pgetvariable_local.VariableName);
+		if (rv)
+			return rv;
+	}
 
-	rv = copy_ucs2_from_user(&name, pgetvariable_local.VariableName);
-	if (rv)
-		return rv;
+	pattr = pgetvariable_local.Attributes ? &attr : NULL;
+	pdatasize = pgetvariable_local.DataSize ? &datasize : NULL;
 
-	data = kmalloc(datasize, GFP_KERNEL);
-	if (!data) {
-		ucs2_kfree(name);
-		return -ENOMEM;
+	if (pgetvariable_local.Data) {
+		data = kmalloc(datasize, GFP_KERNEL);
+		if (!data) {
+			ucs2_kfree(name);
+			return -ENOMEM;
+		}
 	}
 
 	prev_datasize = datasize;
-	status = efi.get_variable(name, &vendor, &attr, &datasize, data);
+	status = efi.get_variable(name, pvendor, pattr, pdatasize, data);
 	ucs2_kfree(name);
 
-	if (status == EFI_SUCCESS && prev_datasize >= datasize)
-		rv = copy_to_user(pgetvariable_local.Data, data, datasize);
-	kfree(data);
+	if (data) {
+		if (status == EFI_SUCCESS && prev_datasize >= datasize)
+			rv = copy_to_user(pgetvariable_local.Data, data, datasize);
+		kfree(data);
+	}
 
 	if (rv)
 		return rv;
@@ -283,8 +295,9 @@ static long efi_runtime_get_variable(unsigned long arg)
 	if (put_user(status, pgetvariable_local.status))
 		return -EFAULT;
 	if (status == EFI_SUCCESS && prev_datasize >= datasize) {
-		if (put_user(attr, pgetvariable_local.Attributes) ||
-		    put_user(datasize, pgetvariable_local.DataSize))
+		if (pattr && put_user(attr, pgetvariable_local.Attributes))
+			return -EFAULT;
+		if (pdatasize && put_user(datasize, pgetvariable_local.DataSize))
 			return -EFAULT;
 		return 0;
 	} else {
diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
index ddf3885..0617ff4 100644
--- a/src/uefi/uefirtvariable/uefirtvariable.c
+++ b/src/uefi/uefirtvariable/uefirtvariable.c
@@ -1773,6 +1773,128 @@ static int uefirtvariable_test7(fwts_framework *fw)
 	return FWTS_OK;
 }
 
+static void getvariable_test_invalid(
+	fwts_framework *fw,
+	struct efi_getvariable *getvariable,
+	const char *test)
+{
+	long ioret;
+
+	fwts_log_info(fw, "Testing GetVariable with %s.", test);
+
+	ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, getvariable);
+	if (ioret == -1) {
+		if (*(getvariable->status) == EFI_INVALID_PARAMETER) {
+			fwts_passed(fw, "GetVariable with %s returned error "
+				"EFI_INVALID_PARAMETER as expected.", test);
+			return;
+		}
+		fwts_failed(fw, LOG_LEVEL_HIGH,
+			"UEFIRuntimeGetVariableInvalid",
+			"GetVariable with %s failed to get expected error return "
+			"status, expected EFI_INVALID_PARAMETER.", test);
+		fwts_uefi_print_status_info(fw, *(getvariable->status));
+		return;
+	}
+	fwts_failed(fw, LOG_LEVEL_HIGH,
+		"UEFIRuntimeGetVariableInvalid",
+		"GetVariable wuth %s failed to get an error return status, "
+		"expected EFI_INVALID_PARAMETER.", test);
+
+	return;
+
+}
+
+static int uefirtvariable_test8(fwts_framework *fw)
+{
+	struct efi_getvariable getvariable;
+	struct efi_setvariable setvariable;
+	uint8_t data[16];
+	uint64_t status, dataindex;
+	uint64_t getdatasize = sizeof(data);
+	uint32_t attr;
+	int ioret;
+
+	for (dataindex = 0; dataindex < sizeof(data); dataindex++)
+		data[dataindex] = (uint8_t)dataindex;
+
+	setvariable.VariableName = variablenametest;
+	setvariable.VendorGuid = &gtestguid1;
+	setvariable.Attributes = attributes;
+	setvariable.DataSize = sizeof(data);
+	setvariable.Data = data;
+	setvariable.status = &status;
+
+	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
+	if (ioret == -1) {
+		if (status == EFI_OUT_OF_RESOURCES) {
+			fwts_uefi_print_status_info(fw, status);
+			fwts_skipped(fw,
+				"Run out of resources for SetVariable UEFI "
+				"runtime interface: cannot test.");
+			fwts_advice(fw,
+				"Firmware may reclaim some resources after "
+				"rebooting. Reboot and test again may be "
+				"helpful to continue the test.");
+			return FWTS_SKIP;
+		}
+		fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeSetVariable",
+			"Failed to set variable with UEFI runtime service.");
+		return FWTS_ERROR;
+	}
+
+	getvariable.VariableName = NULL;
+	getvariable.VendorGuid = &gtestguid1;
+	getvariable.Attributes = &attr;
+	getvariable.DataSize = &getdatasize;
+	getvariable.Data = data;
+	getvariable.status = &status;
+	getvariable_test_invalid(fw, &getvariable, "NULL variable name");
+
+	getvariable.VariableName = variablenametest;
+	getvariable.VendorGuid = NULL;
+	getvariable.Attributes = &attr;
+	getvariable.DataSize = &getdatasize;
+	getvariable.Data = data;
+	getvariable.status = &status;
+	getvariable_test_invalid(fw, &getvariable, "NULL vendor GUID");
+
+	getvariable.VariableName = variablenametest;
+	getvariable.VendorGuid = &gtestguid1;
+	getvariable.Attributes = &attr;
+	getvariable.DataSize = NULL;
+	getvariable.Data = data;
+	getvariable.status = &status;
+	getvariable_test_invalid(fw, &getvariable, "NULL datasize");
+
+	getvariable.VariableName = variablenametest;
+	getvariable.VendorGuid = &gtestguid1;
+	getvariable.Attributes = &attr;
+	getvariable.DataSize = &getdatasize;
+	getvariable.Data = NULL;
+	getvariable.status = &status;
+	getvariable_test_invalid(fw, &getvariable, "NULL data");
+
+	getvariable.VariableName = NULL;
+	getvariable.VendorGuid = NULL;
+	getvariable.Attributes = &attr;
+	getvariable.DataSize = NULL;
+	getvariable.Data = NULL;
+	getvariable.status = &status;
+	getvariable_test_invalid(fw, &getvariable, "NULL variable name, vendor GUID, datasize and data");
+
+	/* delete the variable */
+	setvariable.DataSize = 0;
+	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
+	if (ioret == -1) {
+		fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeSetVariable",
+			"Failed to delete variable with UEFI runtime service.");
+		fwts_uefi_print_status_info(fw, status);
+		return FWTS_ERROR;
+	}
+	return FWTS_OK;
+}
+
 static int options_check(fwts_framework *fw)
 {
 	FWTS_UNUSED(fw);
@@ -1843,6 +1965,7 @@ static fwts_framework_minor_test uefirtvariable_tests[] = {
 	{ uefirtvariable_test5, "Test UEFI RT service variable interface stress test." },
 	{ uefirtvariable_test6, "Test UEFI RT service set variable interface stress test." },
 	{ uefirtvariable_test7, "Test UEFI RT service query variable info interface stress test." },
+	{ uefirtvariable_test8, "Test UEFI RT service get variable interface, invalid parameters." },
 	{ NULL, NULL }
 };
 
-- 
2.1.4

More information about the fwts-devel mailing list