Re: [Qemu-devel] [PATCH] ppc405_uc: Fix buffer overflow

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 31.08.2012 22:21, schrieb Stefan Weil:
>> Report from smatch:
>> 
>> ppc405_uc.c:209 dcr_read_pob(12) error: buffer overflow 'pob->besr' 2 <= 2
>> ppc405_uc.c:232 dcr_write_pob(12) error: buffer overflow 'pob->besr' 2 <= 2
>> 
>> The old code reads and writes besr[POB0_BESR1 - POB0_BESR0] or besr[2]
>> which is one too much.
>> 
>> Signed-off-by: Stefan Weil <address@hidden>
>> ---
>> 
>> As this code was wrong for more than 5 years, there is no urgent need to
>> fix it now for QEMU 1.2.
>> 
>> Regards,
>> 
>> Stefan Weil
>> 
>>  hw/ppc405_uc.c |   16 +++++++++++-----
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>> 
>> diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c
>> index 89e5013..b52ab2f 100644
>> --- a/hw/ppc405_uc.c
>> +++ b/hw/ppc405_uc.c
>> @@ -191,7 +191,8 @@ enum {
>>  typedef struct ppc4xx_pob_t ppc4xx_pob_t;
>>  struct ppc4xx_pob_t {
>>      uint32_t bear;
>> -    uint32_t besr[2];
>> +    uint32_t besr0;
>> +    uint32_t besr1;
>>  };
>>  
>>  static uint32_t dcr_read_pob (void *opaque, int dcrn)
>
> Reviewed-by: Andreas Färber <address@hidden>
>
> We could alternatively leave besr[2] and access it with hardcoded 0..1.

Minimally invasive fix would be besr[dcrn != POB0_BESR0].

[...]