Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • xserver xserver
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 936
    • Issues 936
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 100
    • Merge requests 100
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • xorg
  • xserverxserver
  • Issues
  • #1270

Closed
Open
Created Dec 16, 2021 by tholin .@tholinContributor

Incomplete fix for race condition in GetPairedDevice?

When I was searching for race conditions in dix/devices.c I came across this commit.

commit e693c9657f98c334e9921ca2f8ebf710497c0c6a
Author: Arthur Williams <taaparthur@gmail.com>
Date:   Sun Oct 6 11:55:35 2019 -0700

    dix: Check for NULL spriteInfo in GetPairedDevice
    
    There is a race when reseting the XServer that causes spriteInfo to be
    NULL in GetPairedDevice resulting a segfault and subsequent crash. The
    problem was noticed when opening a connection, creating master devices,
    destroying master devices and closing the connection during testing.
    
    Signed-off-by: Arthur Williams <taaparthur@gmail.com>

diff --git a/dix/devices.c b/dix/devices.c
index 1b18b168e..00c453980 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -2656,7 +2656,7 @@ GetPairedDevice(DeviceIntPtr dev)
     if (!IsMaster(dev) && !IsFloating(dev))
         dev = GetMaster(dev, MASTER_ATTACHED);
 
-    return dev->spriteInfo->paired;
+    return dev->spriteInfo? dev->spriteInfo->paired: NULL;
 }
 
 /**

The commit tries to fix a race condition but I'm not sure the fix is correct.

spriteInfo is modified in a different thread and can presumably become null at any point. The code tries to make sure spriteInfo is not null before dereferencing but the check and access are not atomic. spriteInfo could become null after the check but before the dereferencing. The commit reduced the timing window of the race but it can still crash.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking