[spice-server] red-replay-qxl: Fix replay on 32 bit systems

Submitted by Frediano Ziglio on July 23, 2019, 8:22 a.m.

Details

Message ID 20190723082230.10381-4-fziglio@redhat.com
State Accepted
Commit c289a6eee31d5ae604ff1dd3dd64452b60257bc1
Headers show
Series "Series without cover letter" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio July 23, 2019, 8:22 a.m.
On 32 systems pointers are 32 bit while QXLPHYSICAL is always
64 bit.
Using pointer -> intptr_t -> QXLPHYSICAL conversion cause pointers
to have higher 32 bit set to 1 if the address is >= 0x80000000.
This is possible depending on address space.
The QXLPHYSICAL is split in 3 sections:
- slot ID;
- generation;
- virtual address.
Current utility using record file (spice-server-replay) set slot ID
and generation to 0 so if the higher bits become all 1 slot ID and
generation won't be 0 causing the utility to fail.
Use pointer -> uintptr_t -> QXLPHYSICAL conversion to avoid this
issue.
Note that for opposite conversion (QXLPHYSICAL_TO_PTR) the conversion
does not change, type is changed just for consistency.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/red-replay-qxl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 674feae2f..b38c4cdbd 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -29,8 +29,8 @@ 
 #include "memslot.h"
 #include "red-parse-qxl.h"
 
-#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(intptr_t)(ptr))
-#define QXLPHYSICAL_TO_PTR(phy) ((void*)(intptr_t)(phy))
+#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(uintptr_t)(ptr))
+#define QXLPHYSICAL_TO_PTR(phy) ((void*)(uintptr_t)(phy))
 
 typedef enum {
     REPLAY_OK = 0,

Comments

ping

> 
> On 32 systems pointers are 32 bit while QXLPHYSICAL is always
> 64 bit.
> Using pointer -> intptr_t -> QXLPHYSICAL conversion cause pointers
> to have higher 32 bit set to 1 if the address is >= 0x80000000.
> This is possible depending on address space.
> The QXLPHYSICAL is split in 3 sections:
> - slot ID;
> - generation;
> - virtual address.
> Current utility using record file (spice-server-replay) set slot ID
> and generation to 0 so if the higher bits become all 1 slot ID and
> generation won't be 0 causing the utility to fail.
> Use pointer -> uintptr_t -> QXLPHYSICAL conversion to avoid this
> issue.
> Note that for opposite conversion (QXLPHYSICAL_TO_PTR) the conversion
> does not change, type is changed just for consistency.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/red-replay-qxl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 674feae2f..b38c4cdbd 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -29,8 +29,8 @@
>  #include "memslot.h"
>  #include "red-parse-qxl.h"
>  
> -#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(intptr_t)(ptr))
> -#define QXLPHYSICAL_TO_PTR(phy) ((void*)(intptr_t)(phy))
> +#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(uintptr_t)(ptr))
> +#define QXLPHYSICAL_TO_PTR(phy) ((void*)(uintptr_t)(phy))
>  
>  typedef enum {
>      REPLAY_OK = 0,
ping

> 
> ping
> 
> > 
> > On 32 systems pointers are 32 bit while QXLPHYSICAL is always
> > 64 bit.
> > Using pointer -> intptr_t -> QXLPHYSICAL conversion cause pointers
> > to have higher 32 bit set to 1 if the address is >= 0x80000000.
> > This is possible depending on address space.
> > The QXLPHYSICAL is split in 3 sections:
> > - slot ID;
> > - generation;
> > - virtual address.
> > Current utility using record file (spice-server-replay) set slot ID
> > and generation to 0 so if the higher bits become all 1 slot ID and
> > generation won't be 0 causing the utility to fail.
> > Use pointer -> uintptr_t -> QXLPHYSICAL conversion to avoid this
> > issue.
> > Note that for opposite conversion (QXLPHYSICAL_TO_PTR) the conversion
> > does not change, type is changed just for consistency.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  server/red-replay-qxl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> > index 674feae2f..b38c4cdbd 100644
> > --- a/server/red-replay-qxl.c
> > +++ b/server/red-replay-qxl.c
> > @@ -29,8 +29,8 @@
> >  #include "memslot.h"
> >  #include "red-parse-qxl.h"
> >  
> > -#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(intptr_t)(ptr))
> > -#define QXLPHYSICAL_TO_PTR(phy) ((void*)(intptr_t)(phy))
> > +#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(uintptr_t)(ptr))
> > +#define QXLPHYSICAL_TO_PTR(phy) ((void*)(uintptr_t)(phy))
> >  
> >  typedef enum {
> >      REPLAY_OK = 0,
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Hi,

Acked-by: Snir Sheriber <ssheribe@redhat.com>

nice catch

Snir.

On 7/23/19 11:22 AM, Frediano Ziglio wrote:
> On 32 systems pointers are 32 bit while QXLPHYSICAL is always
> 64 bit.
> Using pointer -> intptr_t -> QXLPHYSICAL conversion cause pointers
> to have higher 32 bit set to 1 if the address is >= 0x80000000.
> This is possible depending on address space.
> The QXLPHYSICAL is split in 3 sections:
> - slot ID;
> - generation;
> - virtual address.
> Current utility using record file (spice-server-replay) set slot ID
> and generation to 0 so if the higher bits become all 1 slot ID and
> generation won't be 0 causing the utility to fail.
> Use pointer -> uintptr_t -> QXLPHYSICAL conversion to avoid this
> issue.
> Note that for opposite conversion (QXLPHYSICAL_TO_PTR) the conversion
> does not change, type is changed just for consistency.
>
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>   server/red-replay-qxl.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 674feae2f..b38c4cdbd 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -29,8 +29,8 @@
>   #include "memslot.h"
>   #include "red-parse-qxl.h"
>   
> -#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(intptr_t)(ptr))
> -#define QXLPHYSICAL_TO_PTR(phy) ((void*)(intptr_t)(phy))
> +#define QXLPHYSICAL_FROM_PTR(ptr) ((QXLPHYSICAL)(uintptr_t)(ptr))
> +#define QXLPHYSICAL_TO_PTR(phy) ((void*)(uintptr_t)(phy))
>   
>   typedef enum {
>       REPLAY_OK = 0,