[SeaBIOS] [PATCH] display_uuid(): fix incomplete check after the loop
Amos Kong
akong at redhat.com
Fri Dec 21 04:06:32 CET 2012
Hi Laszlo,
> In the v2->v3 change of what would become commit 37676f83
> <http://www.seabios.org/pipermail/seabios/2012-December/005166.html>,
> the
> defense against an initial "addr > end" condition ("wraparound") was
> erroneously loosened.
>
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> src/smbios.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/smbios.c b/src/smbios.c
> index aaa99bc..23713a2 100644
> --- a/src/smbios.c
> +++ b/src/smbios.c
> @@ -569,8 +569,8 @@ display_uuid(void)
> addr += 2;
> }
>
> - /* parsing finished, UUID not found */
> - if (addr == end)
> + /* parsing finished or skipped entirely, UUID not found */
> + if (addr >= end)
It's a stricter check, but I didn't find it will happen.
> return;
>
> uuid = (u8 *)(addr + offsetof(struct smbios_type_1, uuid));
> --
> 1.7.1
| void
| display_uuid(void)
| {
| u32 addr, end;
| u8 *uuid;
| u8 empty_uuid[16] = { 0 };
|
| if (SMBiosAddr == NULL)
| return;
|
| addr = SMBiosAddr->structure_table_address;
| end = addr + SMBiosAddr->structure_table_length;
structure_table_length is unsigned, end always >= addr here.
| /* the following takes care of any initial wraparound too */
| while (addr < end) {
| const struct smbios_structure_header *hdr;
|
| /* partial structure header */
| if (end - addr < sizeof(struct smbios_structure_header))
| return;
|
| hdr = (struct smbios_structure_header *)addr;
|
| /* partial structure */
| if (end - addr < hdr->length)
| return;
|
| /* any Type 1 structure version will do that has the UUID */
| if (hdr->type == 1 &&
| hdr->length >= offsetof(struct smbios_type_1, uuid) + 16)
| break;
In this point, it just passes the while condition check, and addr
doesn't change.
* addr < end
|
| /* done with formatted area, skip string-set */
| addr += hdr->length;
|
| while (end - addr >= 2 &&
| (*(u8 *)addr != '\0' ||
| *(u8 *)(addr+1) != '\0'))
| ++addr;
|
| /* structure terminator not found */
| if (end - addr < 2)
| return;
We have above check here, it means if it passes this check
end >= addr + 2
|
| addr += 2;
After above sentence, end >= addr, there is only one condition that
while condition check fails:
* addr = end
| }
The above analysis means that we can only get this point by two
conditions:
* 'break' (addr < end)
* not pass while's condition check (addr >= end)
So the 'addr' will never be larger than 'end'.
However, add this stricter check is harmless.
| /* parsing finished, UUID not found */
| if (addr == end)
| return;
--
Amos.
More information about the SeaBIOS
mailing list