Xorg crashes when it tries to resume a scale transformation after that Screen has been closed

Happens in X.Org X Server 1.20.1 (but also previous versions)

Very easy to reproduce:

• Start a simple X server instance (say Xorg :2 vt8)
• Set a scale transformation
  • xrandr -d :2 --output eDP-1 --scale 2x2
• Call xrandr again so that the server tries to resume the previous transformation
  • xrandr -d :2

The stacktrace is:

#0  0x00007ffff6400f11 in __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:62                                                                
#1  0x000055555566bc12 in transform_filter_length (transform=0x555555d75880, transform=0x555555d75880) at ../../../../randr/rrcrtc.c:1735                 
#2  0x000055555566bc12 in ProcRRGetCrtcTransform (client=0x555555b28ca0) at ../../../../randr/rrcrtc.c:1795                                               
#3  0x00005555555ae91e in Dispatch () at ../../../../dix/dispatch.c:478
#4  0x00005555555b28c6 in dix_main (argc=3, argv=0x7fffffffe668, envp=<optimized out>) at ../../../../dix/main.c:276                                      
#5  0x00007ffff62a109b in __libc_start_main (main=
    0x55555559c640 <main>, argc=3, argv=0x7fffffffe668, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe658)  
    at ../csu/libc-start.c:308
#6  0x000055555559c67a in _start ()

(gdb) f 1
#1  0x000055555566bc12 in transform_filter_length (transform=0x555555d75880, transform=0x555555d75880) at ../../../../randr/rrcrtc.c:1735                 
1735    ../../../../randr/rrcrtc.c: No such file or directory.
(gdb) print *transform
$1 = {transform = {matrix = {{131072, 0, 0}, {0, 131072, 0}, {0, 0, 65536}}}, f_transform = {m = {{2, 0, 0}, {0, 2, 0}, {0, 0, 1}}}, f_inverse = {m = {{  
        0.5, -0, 0}, {-0, 0.5, -0}, {0, -0, 1}}}, filter = 0x5555559aa450, params = 0x0, nparams = 0, width = 2, height = 2}                              
(gdb) print *transform->filter
$2 = {name = 0x0, id = 667696, ValidateParams = 0x555555df1330, width = -199032448, height = 32767}

But this doesn't say much more, other than that's a memory error.

After some debugging, basically what happens is that when the screen get closed, PictureCloseScreen is called and that causes PictureResetFilters to unset the pointers to the filters we've assigned to the crtc on first call.

This debugging log should make it quite clear:

INITTING current transform (nil)                                                                                                               [1063/10642]
INITTING current transform (nil)
INITTING current transform (nil)
INITTING current transform (nil)
COMPIUTING current transform (nil)
Checking filter value for (nil)
Checking filter value for (nil)
RRTransformSetFilter dest 0x55e41b05edf8 filter (nil), filter 0x55e41b05edf8
New dest 0x55e41b05edf8 filter is 0x55e41b05e4e0
Setting CLient pending transform filter to 0x55e41b05e4e0
RRTransformCopy 0x55e41b05edf8->0x55e41b05e4e0 => 0x55e41b052940->(nil)
RRTransformSetFilter dest 0x55e41b052940 filter (nil), filter 0x55e41b052940
New dest 0x55e41b052940 filter is 0x55e41b05e4e0
--->RRTransformCopy 0x55e41b05edf8->0x55e41b05e4e0 => 0x55e41b052940->0x55e41b05e4e0
Copying 0x55e41b05e4e0 in current transform (nil)
RRTransformCopy 0x55e41b052940->0x55e41b05e4e0 => 0x55e41b05eed0->(nil)
RRTransformSetFilter dest 0x55e41b05eed0 filter (nil), filter 0x55e41b05eed0
New dest 0x55e41b05eed0 filter is 0x55e41b05e4e0
--->RRTransformCopy 0x55e41b052940->0x55e41b05e4e0 => 0x55e41b05eed0->0x55e41b05e4e0
COMPIUTING current transform 0x55e41b05e4e0
ProcXIGrabDevice
Temp grab is 0x55e41b4bdfa0
Freeeing temp grab 0x55e41b4bdfa0
Grabdevice return 1, status 0
Ungrabbing grab for dev 0x55e41b1d9fc0 (3), Virtual core keyboard. Grab is 0x55e41b4c6510


xf86CrtcCloseScreen
CLOSING screen, crtc transform 0x55e41b052940->0x55e41b05e4e0
Reset FIlters 0x55e41b05e4c0
Freeing filter 0x55e41b05e4c0 (nearest)
Freeing filter 0x55e41b05e4e0 (bilinear)
Freeing filter 0x55e41b05e500 (convolution)
PictureAddFilter 0x55e41b508940 (nearest)
PictureAddFilter 0x55e41b508960 (bilinear)
PictureAddFilter 0x55e41b508600 (convolution)
INITTING current transform (nil)
INITTING current transform (nil)
INITTING current transform (nil)
INITTING current transform (nil)
RRTransformCopy 0x55e41b052a20->0x55e41b05e4e0 => 0x55e41b052940->(nil)
RRTransformSetFilter dest 0x55e41b052940 filter (nil), filter 0x55e41b052940
New dest 0x55e41b052940 filter is 0x55e41b05e4e0
--->RRTransformCopy 0x55e41b052a20->0x55e41b05e4e0 => 0x55e41b052940->0x55e41b05e4e0
Copying 0x55e41b05e4e0 in current transform (nil)
RRTransformCopy 0x55e41b052940->0x55e41b05e4e0 => 0x55e41b3d2280->(nil)
RRTransformSetFilter dest 0x55e41b3d2280 filter (nil), filter 0x55e41b3d2280
New dest 0x55e41b3d2280 filter is 0x55e41b05e4e0
--->RRTransformCopy 0x55e41b052940->0x55e41b05e4e0 => 0x55e41b3d2280->0x55e41b05e4e0
COMPUTING current transform 0x55e41b05e4e0

Checking filter value for (nil)                                                                                                                           
Checking filter value for 0x55e41b05e4e0 

If you follow the 0x55e41b05e4e0 pointer is clear that it gets free'd on screen closing, but it's sitll assigned to a crtc, and thus that is going to be reused at later times.

One simple solution would be to unset the requested transformation, with something like

diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 37a45bb3a..177da035c 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
         xf86CrtcPtr crtc = config->crtc[c];
 
         crtc->randr_crtc = NULL;
+
+        if (crtc->desiredTransformPresent) {
+            crtc->desiredTransformPresent = FALSE;
+            RRTransformFini(&crtc->desiredTransform);
+            RRTransformInit(&crtc->desiredTransform);
+        }
     }
 
     screen->CloseScreen = config->CloseScreen;

But this has the bad side effect that then the transformation that was previously set before the close is lost, and I guess we don't this either.

However, I guess the best would be to keep a copy of the filter information before closing the screen, and destroying them on the crtc destruction, but this is a bit unclean to do without breaking the ABI, which again isn't something that should be done for this.