[Spice-devel] [xf86 qxl driver PATCH 5/5] qxl_driver: monitors_config: adjust to memory-remap

[Uri Lublin]

On 01/21/2013 04:16 PM, Yonit Halperin wrote:
> Hi,
> On 01/17/2013 09:26 AM, Uri Lublin wrote:
>> Resolves: rhbz#883578
>>
>> Call qxl_allocate_monitors_config upon memory-remap such
>> that qxl->monitors_config points to the start of
>> monitors_config segment in qxl RAM memory.
>>
>> Currently after memory remap, it's possible that monitors_config
>> memory and video-memory (or graphics) overlap, which means
>> that one may overwrite another.
>> Specifically in the bug above, monitors_config value are being
>> overwritten by video pages, and on the destination bad values
>> are read which cause problems on the server and client.
>>
> Can you please explain the path that leads to this overwriting?
> I see that in qxl_map_memory qxl_allocate_monitors_config is already 
> called.

Hi Yonit,

Sorry, I missed this comment earlier.

Before this patch, qxl->monitors_config was set once when the first
memory mapping is done. Upon memory re-map it's possible (even
likely) that qxl->ram gets a different address. But qxl->monitors_config
still points to the first area. Which is probably allocated/mapped
to a different purpose, for example video-pages.


Lets assume the following:
   qxl ram size                         64MB is divided into:
     surface0 area size                          16MB               
(0x1000000)
     Video Pages (aka command ram)  48MB - 12KB   (0x2FFD000)
     monitors config                                 4KB               
(0x0001000)
     qxl ram header                                  8KB              
(0x0002000)

Assume that after the first mapping qxl->ram is        0x7F9C53350000
               and after the second mapping qxl->ram is   0x7F9C53400000

In this case monitors_config will point to 0x7F9C5734D000
     0x7F9C53350000
+           0x1000000
+           0x2FFD000
     -----------------------
     0x7F9C5734D000

and after memory remap Video Pages region will start at
    0x7F9C53400000
+          0x1000000
    -----------------------
    0x7F9C54400000

and end at
     0x7F9C53400000
+           0x1000000
+           0x2FFD000
     -----------------------
     0x7F9C573FD000

So after memory re-map, memory_config points to somewhere in the Video 
Pages area.

This is just an example.
I think that in any case where qxl->ram pointer changes upon memory 
re-map, there
will be memory corruption.

Thanks,
     Uri.