| Message ID | 20180227194816.17940-1-ppandit@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | multiboot: check mh_load_end_addr address field | expand |
+-- On Wed, 28 Feb 2018, P J P wrote --+
| While loading kernel via multiboot-v1 image, (flags & 0x00010000)
| indicates that multiboot header contains valid addresses to load
| the kernel image. In that, end of the data segment address
| 'mh_load_end_addr' should be less than the bss segment address
| 'mh_bss_end_addr'. Add check to validate that.
|
| + if (mh_load_end_addr > mh_bss_end_addr) {
| + fprintf(stderr, "invalid mh_load_end_addr address\n");
| + exit(1);
| + }
|
Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Hi Prasad. The Multiboot Spec will allow for a zero bss end address. (Please see section 3.1.3 at https://www.gnu.org/software/grub/manual/multiboot/ . ) For a zero bss end address, this patch will not do the right thing. I had proposed some patches to properly handle zero bss end address, but they got delayed. However, I will be re-sending updated patches out later today. Please stay tuned... Thanks, Jack On 02/27/18 11:48, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While loading kernel via multiboot-v1 image, (flags & 0x00010000) > indicates that multiboot header contains valid addresses to load > the kernel image. In that, end of the data segment address > 'mh_load_end_addr' should be less than the bss segment address > 'mh_bss_end_addr'. Add check to validate that. > > Reported-by: CERT CC <cert.cc@orange.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/i386/multiboot.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c > index 46d9c68bf5..d16e32bf4a 100644 > --- a/hw/i386/multiboot.c > +++ b/hw/i386/multiboot.c > @@ -227,6 +227,10 @@ int load_multiboot(FWCfgState *fw_cfg, > fprintf(stderr, "invalid mh_load_addr address\n"); > exit(1); > } > + if (mh_load_end_addr > mh_bss_end_addr) { > + fprintf(stderr, "invalid mh_load_end_addr address\n"); > + exit(1); > + } > > uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr); > uint32_t mb_load_size = 0;
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c index 46d9c68bf5..d16e32bf4a 100644 --- a/hw/i386/multiboot.c +++ b/hw/i386/multiboot.c @@ -227,6 +227,10 @@ int load_multiboot(FWCfgState *fw_cfg, fprintf(stderr, "invalid mh_load_addr address\n"); exit(1); } + if (mh_load_end_addr > mh_bss_end_addr) { + fprintf(stderr, "invalid mh_load_end_addr address\n"); + exit(1); + } uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr); uint32_t mb_load_size = 0;