From patchwork Thu Aug 29 09:32:44 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 270798 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 26F1F2C00AB for ; Thu, 29 Aug 2013 21:56:10 +1000 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VF0pQ-0002Ne-O8; Thu, 29 Aug 2013 11:55:56 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VEyau-0004ki-Mj for kernel-team@lists.ubuntu.com; Thu, 29 Aug 2013 09:32:48 +0000 Received: from bl16-212-149.dsl.telepac.pt ([188.81.212.149] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1VEyar-000575-SX; Thu, 29 Aug 2013 09:32:46 +0000 From: Luis Henriques To: Matt Fleming Subject: [ 3.5.y.z extended stable ] Patch "efivars: explicitly calculate length of VariableName" has been added to staging queue Date: Thu, 29 Aug 2013 10:32:44 +0100 Message-Id: <1377768764-11146-1-git-send-email-luis.henriques@canonical.com> X-Mailer: git-send-email 1.8.3.2 X-Extended-Stable: 3.5 X-Mailman-Approved-At: Thu, 29 Aug 2013 11:55:55 +0000 Cc: Lingzhu Xiang , Josh Boyer , Lee Chun-Yi , kernel-team@lists.ubuntu.com, Seiji Aguchi , Frederic Crozat , Michael Schroeder X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com This is a note to let you know that I have just added a patch titled efivars: explicitly calculate length of VariableName to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree which can be found at: http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.5.y-queue If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 3.5.y.z tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Luis ------ From f3536b5cd6edbd0ea4059899c5e43adf8c2b22d0 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 1 Mar 2013 14:49:12 +0000 Subject: [PATCH] efivars: explicitly calculate length of VariableName commit ec50bd32f1672d38ddce10fb1841cbfda89cfe9a upstream. It's not wise to assume VariableNameSize represents the length of VariableName, as not all firmware updates VariableNameSize in the same way (some don't update it at all if EFI_SUCCESS is returned). There are even implementations out there that update VariableNameSize with values that are both larger than the string returned in VariableName and smaller than the buffer passed to GetNextVariableName(), which resulted in the following bug report from Michael Schroeder, > On HP z220 system (firmware version 1.54), some EFI variables are > incorrectly named : > > ls -d /sys/firmware/efi/vars/*8be4d* | grep -v -- -8be returns > /sys/firmware/efi/vars/dbxDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/KEKDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/SecureBoot-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/SetupMode-Information8be4df61-93ca-11d2-aa0d-00e098032b8c The issue here is that because we blindly use VariableNameSize without verifying its value, we can potentially read garbage values from the buffer containing VariableName if VariableNameSize is larger than the length of VariableName. Since VariableName is a string, we can calculate its size by searching for the terminating NULL character. Reported-by: Frederic Crozat Cc: Matthew Garrett Cc: Josh Boyer Cc: Michael Schroeder Cc: Lee, Chun-Yi Cc: Lingzhu Xiang Cc: Seiji Aguchi Signed-off-by: Matt Fleming Cc: Hui Wang [ Hui Wang: backported to 3.5: adjusted context ] Signed-off-by: Luis Henriques --- drivers/firmware/efivars.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) -- 1.8.3.2 diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d7c3540..8c7a8cf 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1006,6 +1006,31 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj, } /* + * Returns the size of variable_name, in bytes, including the + * terminating NULL character, or variable_name_size if no NULL + * character is found among the first variable_name_size bytes. + */ +static unsigned long var_name_strnsize(efi_char16_t *variable_name, + unsigned long variable_name_size) +{ + unsigned long len; + efi_char16_t c; + + /* + * The variable name is, by definition, a NULL-terminated + * string, so make absolutely sure that variable_name_size is + * the value we expect it to be. If not, return the real size. + */ + for (len = 2; len <= variable_name_size; len += sizeof(c)) { + c = variable_name[(len / sizeof(c)) - 1]; + if (!c) + break; + } + + return min(len, variable_name_size); +} + +/* * Let's not leave out systab information that snuck into * the efivars driver */ @@ -1232,6 +1257,8 @@ int register_efivars(struct efivars *efivars, &vendor_guid); switch (status) { case EFI_SUCCESS: + variable_name_size = var_name_strnsize(variable_name, + variable_name_size); efivar_create_sysfs_entry(efivars, variable_name_size, variable_name,