From patchwork Wed Mar 6 15:35:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Fleming X-Patchwork-Id: 225532 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 8FB1C2C0388 for ; Thu, 7 Mar 2013 02:35:10 +1100 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1UDGN2-0001eE-AI; Wed, 06 Mar 2013 15:35:08 +0000 Received: from arkanian.console-pimps.org ([212.110.184.194]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1UDGN0-0001e4-Ns for fwts-devel@lists.ubuntu.com; Wed, 06 Mar 2013 15:35:06 +0000 Received: by arkanian.console-pimps.org (Postfix, from userid 1002) id 9DBC86C05D; Wed, 6 Mar 2013 15:35:06 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on arkanian.vm.bytemark.co.uk X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED autolearn=unavailable version=3.3.1 Received: from localhost (unknown [151.224.133.121]) by arkanian.console-pimps.org (Postfix) with ESMTPSA id 079C56C055; Wed, 6 Mar 2013 15:35:04 +0000 (GMT) From: Matt Fleming To: fwts-devel@lists.ubuntu.com Subject: [PATCH v2] uefirtvariable: Check new VariableNameSize from GetNextVariableName() Date: Wed, 6 Mar 2013 15:35:03 +0000 Message-Id: <1362584103-21521-1-git-send-email-matt@console-pimps.org> X-Mailer: git-send-email 1.7.11.7 Cc: Matt Fleming 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: Matt Fleming Some firmware implementations update VariableNameSize in GetNextVariableName() with a value that is larger than the actual buffer required to hold the VariableName string. This is not technically a bug, but most implementations either update VariableNameSize with the value of strlen(VariableName) + 1 or leave it as the initial value passed to GetNextVariableName(). Print a warning if a different value is found. Signed-off-by: Matt Fleming Acked-by: Colin Ian King Acked-by: Ivan Hu --- v2: Don't warn if variablenamesize wasn't updated at all. src/uefi/uefirtvariable/uefirtvariable.c | 74 +++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c index e00b343..50d6a36 100644 --- a/src/uefi/uefirtvariable/uefirtvariable.c +++ b/src/uefi/uefirtvariable/uefirtvariable.c @@ -226,7 +226,7 @@ static bool compare_name(uint16_t *name1, uint16_t *name2) return ident; } -static int getnextvariable_test(fwts_framework *fw) +static int getnextvariable_test1(fwts_framework *fw) { long ioret; uint64_t status; @@ -339,6 +339,72 @@ err_restore_env: return FWTS_ERROR; } +/* + * Return true if variablenamesize is the length of the + * NULL-terminated unicode string, variablename. + */ +static bool strlen_valid(uint16_t *variablename, uint64_t variablenamesize) +{ + uint64_t len; + uint16_t c; + + for (len = 2; len <= variablenamesize; len += sizeof(c)) { + c = variablename[(len / sizeof(c)) - 1]; + if (!c) + break; + } + + return len == variablenamesize; +} + +static int getnextvariable_test2(fwts_framework *fw) +{ + long ioret; + uint64_t status; + + struct efi_getnextvariablename getnextvariablename; + uint64_t variablenamesize = MAX_DATA_LENGTH; + uint16_t variablename[MAX_DATA_LENGTH]; + EFI_GUID vendorguid; + + getnextvariablename.VariableNameSize = &variablenamesize; + getnextvariablename.VariableName = variablename; + getnextvariablename.VendorGuid = &vendorguid; + getnextvariablename.status = &status; + + /* To start the search, need to pass a Null-terminated string in VariableName */ + variablename[0] = '\0'; + while (true) { + variablenamesize = MAX_DATA_LENGTH; + ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename); + + if (ioret == -1) { + + /* no next variable was found*/ + if (*getnextvariablename.status == EFI_NOT_FOUND) + break; + + fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextVariableName", + "Failed to get next variable name with UEFI runtime service."); + fwts_uefi_print_status_info(fw, status); + goto err; + } + + if (variablenamesize != MAX_DATA_LENGTH && + !strlen_valid(variablename, variablenamesize)) { + fwts_warning(fw, "UEFIRuntimeGetNextVariableName " + "Unexpected variable name size returned."); + goto err; + } + }; + + return FWTS_OK; + +err: + + return FWTS_ERROR; +} + static int setvariable_insertvariable(fwts_framework *fw, uint32_t attributes, uint64_t datasize, uint16_t *varname, EFI_GUID *gtestguid, uint8_t datadiff) { @@ -857,7 +923,11 @@ static int uefirtvariable_test2(fwts_framework *fw) { int ret; - ret = getnextvariable_test(fw); + ret = getnextvariable_test1(fw); + if (ret != FWTS_OK) + return ret; + + ret = getnextvariable_test2(fw); if (ret != FWTS_OK) return ret;