lkml.org 
[lkml]   [2013]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 50/58] efivars: explicitly calculate length of VariableName
    Date
    3.5.7.21 -stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Matt Fleming <matt.fleming@intel.com>

    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 <fcrozat@suse.com>
    Cc: Matthew Garrett <mjg59@srcf.ucam.org>
    Cc: Josh Boyer <jwboyer@redhat.com>
    Cc: Michael Schroeder <mls@suse.com>
    Cc: Lee, Chun-Yi <jlee@suse.com>
    Cc: Lingzhu Xiang <lxiang@redhat.com>
    Cc: Seiji Aguchi <seiji.aguchi@hds.com>
    Signed-off-by: Matt Fleming <matt.fleming@intel.com>
    Cc: Hui Wang <hui.wang@canonical.com>
    [ Hui Wang: backported to 3.5: adjusted context ]
    Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
    ---
    drivers/firmware/efivars.c | 27 +++++++++++++++++++++++++++
    1 file changed, 27 insertions(+)

    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,
    --
    1.8.3.2


    \
     
     \ /
      Last update: 2013-09-02 15:21    [W:4.248 / U:0.264 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site