Re: [Qemu-devel] [PATCH v4 4/6] fw_cfg: avoid calculating invalid current entry pointer

[Laszlo Ersek <lersek@redhat.com>]

On 11/03/15 22:40, Gabriel L. Somlo wrote:
> When calculating a pointer to the currently selected fw_cfg item, the
> following is used:
> 
>   FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
> 
> When s->cur_entry is FW_CFG_INVALID, we are calculating the address of
> a non-existent element in s->entries[arch][...], which is undefined.
> 
> This patch ensures the resulting entry pointer is se to NULL whenever

se[t] to NULL

> s->cur_entry is FW_CFG_INVALID.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Cc: Marc Marí <markmb@redhat.com>
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
>  hw/nvram/fw_cfg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index c2d3a0a..046fa74 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -277,7 +277,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)
> @@ -342,7 +343,8 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
>      }
>  
>      arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
> -    e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
> +    e = (s->cur_entry == FW_CFG_INVALID) ? NULL :
> +        &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
>  
>      if (dma.control & FW_CFG_DMA_CTL_READ) {
>          read = 1;
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>