[libfdisk] incorrect GPT header leads to segfault

[libfdisk] incorrect GPT header leads to segfault

[Otto Visser]

[-- Attachment #1: Type: text/plain, Size: 4767 bytes --]

[note: this is a more or less an adapted copy-paste from
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=780834; now tested
with the source of 2.26.1 instead of with the Debian testing version
2.25.2-5]:


Dear Maintainer,

Let's start with the TL;DR version:
if (lib)fdisk encounters a GPT header with an incorrect size field it
tries to calculate the CRC32 over whatever this size field is
reporting, leading eventually to a segfault.


Longer version:
I'm creating my own hobby OS (including bootloader part; not using GRUB
or anything) and was moving from start execution at first byte of the
HDD to having an actual partition table etc. Instead of using partx to
create the protective MBR and the GPT for my disk image, I decided I
wanted to learn what this GPT looks like and included creating the
MBR/GPT in the Makefile/linker script for the boot loader. I
misinterpreted the part where it said that the size field of the GPT
header is little endian and accidentally created a big endian version,
so my header is not 92 bytes, but a whole lot more. I then thought that
the quickest way to get the CRCs correct(ed) was to probably run fdisk
and let it calculate and fix my CRCs. To my surprise however, it just
segfaulted without any error/warning.


I downloaded the source code (version 2.26.1), compiled with debugging
and got the following trace:

/local/svn/util-linux-2.26.1/.libs$ LD_LIBRARY_PATH=.:$LD_LIBRARY_PATH
gdb ./fdisk
GNU gdb (Debian 7.7.1+dfsg-5) 7.7.1
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./fdisk...done.
(gdb) set args -l /local/OS/img_breaks_fdisk
(gdb) run
Starting program: /local/svn/util-linux-2.26.1/.libs/fdisk -l
/local/OS/img_breaks_fdisk

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7bb5206 in crc32 (seed=4294967295, buf=0x61d260 "EFI PART",
len=1543381600) at lib/crc32.c:112
112                     crc = crc32_tab[(crc ^ *p++) & 0xff] ^ (crc >> 8);
(gdb) bt
#0  0x00007ffff7bb5206 in crc32 (seed=4294967295, buf=0x61d260 "EFI
PART", len=1543381600) at lib/crc32.c:112
#1  0x00007ffff7badc6a in count_crc32 (buf=0x61d260 "EFI PART",
len=1543503872) at libfdisk/src/gpt.c:799
#2  0x00007ffff7badd47 in gpt_check_header_crc (header=0x61d260,
ents=0x0) at libfdisk/src/gpt.c:838
#3  0x00007ffff7bae0ca in gpt_read_header (cxt=0x61a080, lba=1,
_ents=0x61a220) at libfdisk/src/gpt.c:957
#4  0x00007ffff7baec01 in gpt_probe_label (cxt=0x61a080) at
libfdisk/src/gpt.c:1317
#5  0x00007ffff7b8ec5f in fdisk_probe_labels (cxt=0x61a080) at
libfdisk/src/label.c:49
#6  0x00007ffff7b91524 in fdisk_assign_device (cxt=0x61a080,
fname=0x7fffffffe409 "/local/OS/img_breaks_fdisk", readonly=1)
    at libfdisk/src/context.c:528
#7  0x000000000040b527 in print_device_pt (cxt=0x61a080,
device=0x7fffffffe409 "/local/OS/img_breaks_fdisk", warnme=1, verify=0)
    at disk-utils/fdisk-list.c:243
#8  0x00000000004087ed in main (argc=3, argv=0x7fffffffe0c8) at
disk-utils/fdisk.c:832


I then made the following (ordering) change in libfdisk/src/gpt.c:

956a957,961
>       /* make sure header size is between 92 and sector size bytes */
>       hsz = le32_to_cpu(header->size);
>       if (hsz < GPT_HEADER_MINSZ || hsz > cxt->sector_size)
>               goto invalid;
>
973,977d977
<               goto invalid;
<
<       /* make sure header size is between 92 and sector size bytes */
<       hsz = le32_to_cpu(header->size);
<       if (hsz < GPT_HEADER_MINSZ || hsz > cxt->sector_size)

Although this fixes getting the segfault, it still means that fdisk
concludes there is no GPT label, despite that the signature is clearly
there; hence I wouldn't want to call this an actual patch.

Whether this is a patch or not probably boils down to the question how
much of an effort should be done at trying to fix a GPT; my personal
opinion is that an effort should be made if the signature is found; the
user still has the option then to write the suggested changes, start a
new GPT label or quit the program.


Best regards,

Otto Visser.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4201 bytes --]

Re: [libfdisk] incorrect GPT header leads to segfault

[Karel Zak]

On Fri, Mar 20, 2015 at 02:19:05PM +0100, Otto Visser wrote:
> I then made the following (ordering) change in libfdisk/src/gpt.c:
> 
> 956a957,961
> >       /* make sure header size is between 92 and sector size bytes */
> >       hsz = le32_to_cpu(header->size);
> >       if (hsz < GPT_HEADER_MINSZ || hsz > cxt->sector_size)
> >               goto invalid;
> >
> 973,977d977
> <               goto invalid;
> <
> <       /* make sure header size is between 92 and sector size bytes */
> <       hsz = le32_to_cpu(header->size);
> <       if (hsz < GPT_HEADER_MINSZ || hsz > cxt->sector_size)

Thanks. Fixed.

> Although this fixes getting the segfault, it still means that fdisk
> concludes there is no GPT label, despite that the signature is clearly
> there; hence I wouldn't want to call this an actual patch.

Well, be able to work with broken GPT header means that we have to
implement something like fsck for GPT -- analyze each field in the
header and try to fix it. I'm going to add this to TODO.

I don't think something like GPT fsck should be within regular GPT
probing code. It's very special use-case.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com