Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
Hi, Will you assign a cve to this vulnerability.This issue has the possibility to remote code execution, and many IAAS providers use qemu prior version 2.4.Donghai.--From:P J P Send Time:2016年1月6日(星期三) 02:08To:Qemu devel Cc:Stefan Weil ,Peter Maydell ,朱东海(启路) Subject:Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value+-- On Tue, 5 Jan 2016, P J P wrote --+| An OOB r/w access issue was reported by Mr Donghai Zdh, CC'd here.Mr Donghai CC'd now.--Prasad J Pandit / Red Hat Product Security Team47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
+-- On Wed, 6 Jan 2016, 朱东海(启路) wrote --+ | Hi, Will you assign a cve to this vulnerability. Yes, I will once the patch is approved upstream. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
+-- On Tue, 5 Jan 2016, Stefan Weil wrote --+ | > -s->cur_offset < e->len) { | > +if (s->cur_entry != FW_CFG_INVALID | > +&& s->cur_entry & FW_CFG_WRITE_CHANNEL | > +&& e->callback | > +&& s->cur_offset < e->len) { | | I suggest to test e != NULL instead of s->cur_entry != FW_CFG_INVALID. | | Of course both variants are equivalent, but e != NULL might be easier | to review and make work of static code analyzers easier, too. Yes, I've sent a revised patch with this change. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
Am 05.01.2016 um 15:55 schrieb P J P: > From: Prasad J Pandit > > When processing firmware configurations, an OOB r/w access occurs > if 's->cur_entry' is set to be invalid(FW_CFG_INVALID=0x). > Add a check to validate 's->cur_entry' to avoid such access. > > Reported-by: Donghai Zdh > Signed-off-by: Prasad J Pandit > --- > hw/nvram/fw_cfg.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 68eff77..ce026bc 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -233,12 +233,15 @@ static void fw_cfg_reboot(FWCfgState *s) > static void fw_cfg_write(FWCfgState *s, uint8_t value) > { > int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > -FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > +FWCfgEntry *e = (s->cur_entry == FW_CFG_INVALID) ? NULL : > + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > > trace_fw_cfg_write(s, value); > > -if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback && > -s->cur_offset < e->len) { > +if (s->cur_entry != FW_CFG_INVALID > +&& s->cur_entry & FW_CFG_WRITE_CHANNEL > +&& e->callback > +&& s->cur_offset < e->len) { I suggest to test e != NULL instead of s->cur_entry != FW_CFG_INVALID. Of course both variants are equivalent, but e != NULL might be easier to review and make work of static code analyzers easier, too. > e->data[s->cur_offset++] = value; > if (s->cur_offset == e->len) { > e->callback(e->callback_opaque, e->data); > @@ -267,7 +270,8 @@ static int fw_cfg_select(FWCfgState *s, uint16_t key) > static uint8_t fw_cfg_read(FWCfgState *s) > { > int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > -FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > +FWCfgEntry *e = (s->cur_entry == FW_CFG_INVALID) ? NULL : > + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > uint8_t ret; > > if (s->cur_entry == FW_CFG_INVALID || !e->data || s->cur_offset >= > e->len) > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
+-- On Tue, 5 Jan 2016, P J P wrote --+ | An OOB r/w access issue was reported by Mr Donghai Zdh, CC'd here. Mr Donghai CC'd now. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
[Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
From: Prasad J Pandit When processing firmware configurations, an OOB r/w access occurs if 's->cur_entry' is set to be invalid(FW_CFG_INVALID=0x). Add a check to validate 's->cur_entry' to avoid such access. Reported-by: Donghai Zdh Signed-off-by: Prasad J Pandit --- hw/nvram/fw_cfg.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 68eff77..ce026bc 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -233,12 +233,15 @@ static void fw_cfg_reboot(FWCfgState *s) static void fw_cfg_write(FWCfgState *s, uint8_t value) { int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); -FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; +FWCfgEntry *e = (s->cur_entry == FW_CFG_INVALID) ? NULL : + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; trace_fw_cfg_write(s, value); -if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback && -s->cur_offset < e->len) { +if (s->cur_entry != FW_CFG_INVALID +&& s->cur_entry & FW_CFG_WRITE_CHANNEL +&& e->callback +&& s->cur_offset < e->len) { e->data[s->cur_offset++] = value; if (s->cur_offset == e->len) { e->callback(e->callback_opaque, e->data); @@ -267,7 +270,8 @@ static int fw_cfg_select(FWCfgState *s, uint16_t key) static uint8_t fw_cfg_read(FWCfgState *s) { int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); -FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; +FWCfgEntry *e = (s->cur_entry == FW_CFG_INVALID) ? NULL : + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; uint8_t ret; if (s->cur_entry == FW_CFG_INVALID || !e->data || s->cur_offset >= e->len) -- 2.4.3
[Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value
From: Prasad J Pandit Hello, An OOB r/w access issue was reported by Mr Donghai Zdh, CC'd here. It occurs while processing firmware configurations in Qemu versions prior to 2.4. The OOB memory access crashes the Qemu process on the host. Please see below a (tested)patch to fix this issue. Does it look okay? Thank you! Prasad J Pandit (1): fw_cfg: add check to validate current entry value hw/nvram/fw_cfg.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) -- 2.4.3