From patchwork Thu Apr 2 15:59:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 457729 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id DBEA4140079; Fri, 3 Apr 2015 03:01:51 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1YdhZ0-0006RK-Dh; Thu, 02 Apr 2015 16:01:50 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1YdhYu-0006PY-LC for fwts-devel@lists.ubuntu.com; Thu, 02 Apr 2015 16:01:44 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginm.net ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1YdhYu-0004xL-HU for fwts-devel@lists.ubuntu.com; Thu, 02 Apr 2015 16:01:44 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH 1/2] uefi: uefirtvariable: Add invalid NULL parameter sanity checks Date: Thu, 2 Apr 2015 16:59:59 +0100 Message-Id: <1427990400-3374-2-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1427990400-3374-1-git-send-email-colin.king@canonical.com> References: <1427990400-3374-1-git-send-email-colin.king@canonical.com> X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: fwts-devel-bounces@lists.ubuntu.com Sender: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King 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 Acked-by: Alex Hung --- 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 = >estguid1; + 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 = >estguid1; + 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 = >estguid1; + getvariable.Attributes = &attr; + getvariable.DataSize = NULL; + getvariable.Data = data; + getvariable.status = &status; + getvariable_test_invalid(fw, &getvariable, "NULL datasize"); + + getvariable.VariableName = variablenametest; + getvariable.VendorGuid = >estguid1; + 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 } };