linux-kernel - Re: [PATCH v2] efi: take size of partition entry from GPT header

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20181008111525.hm6jcnlsf7xrjre3@ws.net.home>
Date:   Mon, 8 Oct 2018 13:15:25 +0200
From:   Karel Zak <kzak@...hat.com>
To:     Eugene Korenevsky <ekorenevsky@...il.com>
Cc:     David.Laight@...lab.com, Davidlohr Bueso <dave@...olabs.net>,
        linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [PATCH v2] efi: take size of partition entry from GPT header

On Sat, Oct 06, 2018 at 09:41:27PM +0300, Eugene Korenevsky wrote:
> > is_gpt_valid() already contains
> >         pt_size = (u64)le32_to_cpu((*gpt)->num_partition_entries) *
> >                 le32_to_cpu((*gpt)->sizeof_partition_entry);
> >         if (pt_size > KMALLOC_MAX_SIZE)
> >                 pr_debug("GUID Partition Table is too large: %llu > %lu bytes\n",
> >                 (unsigned long long)pt_size, KMALLOC_MAX_SIZE);
> >                 goto fail;
> >         }
> > I guess it good enough for sanity check.
> >
> > If you want to be really paranoid than you can also check that array
> > is possible to store to the expected area on the disk:
> >
> >     pt_size <= (gpt->first_usable_lba - gpt->partition_entry_lba)
> >
> 
> Well, we should apply several checks for different cases:
> - primary GPT: table entries should not override gpt->first_usable_lba

 and gpt->last_usable_lba

> - alternate GPT, table entries BEFORE agpt (agpt->partition_entry_lba
> < agpt_lba): table entries should not override agpt_lba AND
> agpt->partition_entry_lba MUST BE more than agpt->last_usable_lba
> - alternate GPT, table entries AFTER agpt (agpt->partition_entry_lba >
> agpt_lba): table entries should not override the end of the disk
> 
> Is this correct?

Yes, the table defines range for all partitions (last and first usable
LBA). All partition table stuff (label and partitions array) has to be
outside this area and partitions have to point to this area.

                   
 | label | entries |    partitioned area   | backup-entries | backup-label |
 
                   ^                       ^
             first_usable_lba        last_usable_lba


and it's possible and valid if there is gap between entries array and 
first usable LBA (you can use this unused place to hide same data :-) 
And vice-versa for backup entries and last usable LBA. 

    Karel


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