On Tue, Sep 5, 2017 at 11:12 AM, Thomas Garnier <thgar...@google.com> wrote:
> On Tue, Sep 5, 2017 at 10:49 AM, P J P <ppan...@redhat.com> wrote:
>> From: Prasad J Pandit <p...@fedoraproject.org>
>>
>> While loading kernel via multiboot-v1 image, (flags & 0x00010000)
>> indicates that multiboot header contains valid addresses to load
>> the kernel image. These addresses are used to compute kernel
>> size and kernel text offset in the OS image. Validate these
>> address values to avoid an OOB access issue.
>>
>> Reported-by: Thomas Garnier <thgar...@google.com>
>> Signed-off-by: Prasad J Pandit <p...@fedoraproject.org>
>> ---
>
> Looks good, tested.
>
> Tested-by: Thomas Garnier <thgar...@google.com>
Btw, can you open a CVE for that? (and reference it in the commit).
>
>> hw/i386/multiboot.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
>> index 6001f4caa2..c7b70c91d5 100644
>> --- a/hw/i386/multiboot.c
>> +++ b/hw/i386/multiboot.c
>> @@ -221,15 +221,34 @@ int load_multiboot(FWCfgState *fw_cfg,
>> uint32_t mh_header_addr = ldl_p(header+i+12);
>> uint32_t mh_load_end_addr = ldl_p(header+i+20);
>> uint32_t mh_bss_end_addr = ldl_p(header+i+24);
>> +
>> mh_load_addr = ldl_p(header+i+16);
>> + if (mh_header_addr < mh_load_addr) {
>> + fprintf(stderr, "invalid mh_load_addr address\n");
>> + exit(1);
>> + }
>> +
>> uint32_t mb_kernel_text_offset = i - (mh_header_addr -
>> mh_load_addr);
>> uint32_t mb_load_size = 0;
>> mh_entry_addr = ldl_p(header+i+28);
>>
>> if (mh_load_end_addr) {
>> + if (mh_bss_end_addr < mh_load_addr) {
>> + fprintf(stderr, "invalid mh_bss_end_addr address\n");
>> + exit(1);
>> + }
>> mb_kernel_size = mh_bss_end_addr - mh_load_addr;
>> +
>> + if (mh_load_end_addr < mh_load_addr) {
>> + fprintf(stderr, "invalid mh_load_end_addr address\n");
>> + exit(1);
>> + }
>> mb_load_size = mh_load_end_addr - mh_load_addr;
>> } else {
>> + if (kernel_file_size < mb_kernel_text_offset) {
>> + fprintf(stderr, "invalid kernel_file_size\n");
>> + exit(1);
>> + }
>> mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
>> mb_load_size = mb_kernel_size;
>> }
>> --
>> 2.13.5
>>
>
>
>
> --
> Thomas
--
Thomas
