[v1] macfb: fixes for booting MacOS

[PATCH 03/12] macfb: fix overflow of color_palette array

Posted by Mark Cave-Ayland 4 years, 4 months ago

The palette_current index counter has a maximum size of 256 * 3 to cover a full
color palette of 256 RGB entries. Linux assumes that the palette_current index
wraps back around to zero after writing 256 RGB entries so ensure that
palette_current is reset at this point to prevent data corruption within
MacfbState.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/macfb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/display/macfb.c b/hw/display/macfb.c
index 815870f2fe..f4e789d0d7 100644
--- a/hw/display/macfb.c
+++ b/hw/display/macfb.c
@@ -307,6 +307,9 @@ static void macfb_ctrl_write(void *opaque,
         if (s->palette_current % 3) {
             macfb_invalidate_display(s);
         }
+        if (s->palette_current >= sizeof(s->color_palette)) {
+            s->palette_current = 0;
+        }
         break;
     }
 }
-- 
2.20.1

Re: [PATCH 03/12] macfb: fix overflow of color_palette array

Posted by Laurent Vivier 4 years, 4 months ago

Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
> The palette_current index counter has a maximum size of 256 * 3 to cover a full
> color palette of 256 RGB entries. Linux assumes that the palette_current index
> wraps back around to zero after writing 256 RGB entries so ensure that
> palette_current is reset at this point to prevent data corruption within
> MacfbState.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/macfb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 815870f2fe..f4e789d0d7 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -307,6 +307,9 @@ static void macfb_ctrl_write(void *opaque,
>          if (s->palette_current % 3) {
>              macfb_invalidate_display(s);
>          }
> +        if (s->palette_current >= sizeof(s->color_palette)) {
> +            s->palette_current = 0;
> +        }
>          break;
>      }
>  }
> 

What about "s->palette_current = (s->palette_current + 1) % sizeof(s->color_palette)"?

Thanks,
Laurent

Re: [PATCH 03/12] macfb: fix overflow of color_palette array

Posted by Mark Cave-Ayland 4 years, 4 months ago

On 04/10/2021 09:53, Laurent Vivier wrote:

> Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
>> The palette_current index counter has a maximum size of 256 * 3 to cover a full
>> color palette of 256 RGB entries. Linux assumes that the palette_current index
>> wraps back around to zero after writing 256 RGB entries so ensure that
>> palette_current is reset at this point to prevent data corruption within
>> MacfbState.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/display/macfb.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
>> index 815870f2fe..f4e789d0d7 100644
>> --- a/hw/display/macfb.c
>> +++ b/hw/display/macfb.c
>> @@ -307,6 +307,9 @@ static void macfb_ctrl_write(void *opaque,
>>           if (s->palette_current % 3) {
>>               macfb_invalidate_display(s);
>>           }
>> +        if (s->palette_current >= sizeof(s->color_palette)) {
>> +            s->palette_current = 0;
>> +        }
>>           break;
>>       }
>>   }
>>
> 
> What about "s->palette_current = (s->palette_current + 1) % sizeof(s->color_palette)"?
> 
> Thanks,
> Laurent

Sure, I can update that for v2.


ATB,

Mark.