[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/7] smbios: Check R in -smbios type=0, release=
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 7/7] smbios: Check R in -smbios type=0, release=R parses okay |
Date: |
Thu, 06 Jun 2013 22:02:30 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Laszlo Ersek <address@hidden> writes:
> On 06/06/13 18:27, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> hw/i386/smbios.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
>> index 68bd6d0..88a1360 100644
>> --- a/hw/i386/smbios.c
>> +++ b/hw/i386/smbios.c
>> @@ -140,7 +140,10 @@ static void smbios_build_type_0_fields(const char *t)
>> bios_release_date_str),
>> buf, strlen(buf) + 1);
>> if (get_param_value(buf, sizeof(buf), "release", t)) {
>> - sscanf(buf, "%hhd.%hhd", &major, &minor);
>> + if (sscanf(buf, "%hhd.%hhd", &major, &minor) != 2) {
>> + error_report("Invalid release");
>> + exit(1);
>> + }
>> smbios_add_field(0, offsetof(struct smbios_type_0,
>> system_bios_major_release),
>> &major, 1);
>>
>
> Right. OTOH if any of the decimal strings provided doesn't fit into the
> space provided (eg. you pass "256" for an "unsigned char" which happens
> to be uint8_t), the behavior is undefined anyway. sscanf() cannot be
> used with "untrusted" data. ("... if the result of the conversion cannot
> be represented in the space provided, the behavior is undefined.")
Yes, this isn't rigorous parsing. It improves the code from "brazenly
careless" to the more common (in QEMU) "quick but sloppy".
> Reviewed-by: Laszlo Ersek <address@hidden>
Thanks for the review!
- [Qemu-devel] [PATCH 2/7] log.h: Supply missing includes, (continued)
- [Qemu-devel] [PATCH 2/7] log.h: Supply missing includes, Markus Armbruster, 2013/06/06
- [Qemu-devel] [PATCH 4/7] Use sizeof(qemu_uuid) instead of literal 16, Markus Armbruster, 2013/06/06
- [Qemu-devel] [PATCH 6/7] smbios: Fix -smbios type=0, release=... for big endian hosts, Markus Armbruster, 2013/06/06
- [Qemu-devel] [PATCH 7/7] smbios: Check R in -smbios type=0, release=R parses okay, Markus Armbruster, 2013/06/06
- [Qemu-devel] [PATCH 3/7] smbios: Convert to error_report(), Markus Armbruster, 2013/06/06
- [Qemu-devel] [PATCH 5/7] smbios: Clean up smbios_add_field() parameters, Markus Armbruster, 2013/06/06