| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54C62ACE.1080503@linaro.org>
Date: Mon, 26 Jan 2015 13:53:50 +0200
From: Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
To: Ard Biesheuvel <ard.biesheuvel@...aro.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
dmidecode-devel@...gnu.org, Grant Likely <grant.likely@...aro.org>,
Leif Lindholm <leif.lindholm@...aro.org>,
Matt Fleming <matt.fleming@...el.com>,
"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH 1/2] firmware: dmi_scan: add symbol to get SMBIOS entry
area
On 01/26/2015 10:44 AM, Ard Biesheuvel wrote:
> On 23 January 2015 at 20:21, Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org> wrote:
>> There are situations when code needs to access SMBIOS entry table
>> area. For example, to pass it via sysfs to userspace when it's not
>> allowed to get SMBIOS info via /dev/mem.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
>> ---
>> drivers/firmware/dmi_scan.c | 34 ++++++++++++++++++++++++++++++++++
>> include/linux/dmi.h | 2 ++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
>> index 420c8d8..ae9204a 100644
>> --- a/drivers/firmware/dmi_scan.c
>> +++ b/drivers/firmware/dmi_scan.c
>> @@ -113,6 +113,7 @@ static void dmi_table(u8 *buf, int len, int num,
>> }
>> }
>>
>> +static unsigned char smbios_header[32];
>> static phys_addr_t dmi_base;
>> static u16 dmi_len;
>> static u16 dmi_num;
>> @@ -474,6 +475,7 @@ static int __init dmi_present(const u8 *buf)
>> if (memcmp(buf, "_SM_", 4) == 0 &&
>> buf[5] < 32 && dmi_checksum(buf, buf[5])) {
>> smbios_ver = get_unaligned_be16(buf + 6);
>> + memcpy(smbios_header, buf, buf[5]);
>>
>> /* Some BIOS report weird SMBIOS version, fix that up */
>> switch (smbios_ver) {
>> @@ -505,6 +507,7 @@ static int __init dmi_present(const u8 *buf)
>> pr_info("SMBIOS %d.%d present.\n",
>> dmi_ver >> 8, dmi_ver & 0xFF);
>> } else {
>> + memcpy(smbios_header, buf, 15);
>> dmi_ver = (buf[14] & 0xF0) << 4 |
>> (buf[14] & 0x0F);
>> pr_info("Legacy DMI %d.%d present.\n",
>> @@ -531,6 +534,7 @@ static int __init dmi_smbios3_present(const u8 *buf)
>> dmi_ver &= 0xFFFFFF;
>> dmi_len = get_unaligned_le32(buf + 12);
>> dmi_base = get_unaligned_le64(buf + 16);
>> + memcpy(smbios_header, buf, buf[6]);
>>
>> /*
>> * The 64-bit SMBIOS 3.0 entry point no longer has a field
>> @@ -944,3 +948,33 @@ void dmi_memdev_name(u16 handle, const char **bank, const char **device)
>> }
>> }
>> EXPORT_SYMBOL_GPL(dmi_memdev_name);
>> +
>> +/**
>> + * dmi_get_smbios_entry_area - copy SMBIOS entry point area to array.
>> + * @entry - pointer on array to read area in, current max size is 32 bytes.
>> + *
>> + * returns -ENODATA if table is not available, otherwise returns actual
>> + * size of SMBIOS entry point area.
>> + */
>> +int dmi_get_smbios_entry_area(char *table)
>> +{
> What about
>
> const u8 *dmi_get_smbios_header(int *size)
>
> and return the buffer (or NULL if no data) and the size previously
> recorded in *size (see below)
>
> The reason is that, in the second patch, you are copying the data into
> yet another char[32], which doesn't make a great deal of sense is the
> data is not captured right at that time
>
> (I suggested earlier that the correct way to implement this was to
> populate the header at the same time you traverse the dmi tree to
> populate the sysfs entries. If you capture the data at init time,
> there is no reason to copy it yet again at dmi_sysfs module_init time)
yes. Why I don't trust to return pointer even in case of const.
Even don't remember.
I'll update with const.
Thanks.
>> + int size = 0;
>> +
>> + if (!dmi_available)
>> + return -ENODATA;
>> +
>> + if (memcmp(smbios_header, "_SM3_", 5) == 0)
>> + size = smbios_header[6];
>> + else if (memcmp(smbios_header, "_SM_", 4) == 0)
>> + size = smbios_header[5];
>> + else if (memcmp(smbios_header, "_DMI_", 5) == 0)
>> + size = 15;
>> +
> You are duplicating work here: could we record smbios_header_size when
> you capture the data itself?
~
I'll update soon.
>
>> + memcpy(table, smbios_header, size);
>> +
>> + if (!size)
>> + return -ENODATA;
>> +
>> + return size;
>> +}
>> +EXPORT_SYMBOL_GPL(dmi_get_smbios_entry_area);
>> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
>> index f820f0a..7cae713 100644
>> --- a/include/linux/dmi.h
>> +++ b/include/linux/dmi.h
>> @@ -109,6 +109,7 @@ extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>> void *private_data);
>> extern bool dmi_match(enum dmi_field f, const char *str);
>> extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
>> +int dmi_get_smbios_entry_area(char *table);
>>
>> #else
>>
>> @@ -140,6 +141,7 @@ static inline void dmi_memdev_name(u16 handle, const char **bank,
>> const char **device) { }
>> static inline const struct dmi_system_id *
>> dmi_first_match(const struct dmi_system_id *list) { return NULL; }
>> +static inline int dmi_get_smbios_entry_area(char *table) { return -ENODATA; }
>>
>> #endif
>>
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists