PowerVR Patch Security Issues

While reviewing a preview patch for https://bugs.chromium.org/p/project-zero/issues/detail?id=2540 , I noticed some issues - most of them minor, but the following two seem like they probably have bigger security impact:
    
    
    ** F.5 **
    After _PmrZombieCleanup() has picked an item from the sZombieList, it
    drops the hPMRZombieListLock temporarily to avoid a deadlock. When the
    hPMRZombieListLock has been retaken, _PmrZombieCleanup() tries to make
    sure that the PMR is still a zombie, but it does that by checking
    PMR_IsZombie(); so if another thread concurrently revives the zombie
    with PMRDequeueZombieAndRef(), maps it on the GPU, unmaps it, and
    zombifies it again, _PmrZombieCleanup() could wrongly assume that the
    PMR stayed a zombie while the lock was dropped, and destroy it
    immediately.
    In other words, the problem is that after retaking the lock,
    _PmrZombieCleanup() doesn't check whether the zombie is still on the
    same list as before. Instead of rechecking PMR_IsZombie(), you might
    want to recheck that
    `dllist_get_next_node(&psCleanupItem->sZombieList) == psNode`.
    It might also be nice to have a comment in _PmrZombieCleanup() describing this.
    
    
    ** F.9 **
    The refactor of the bools in `struct _PMR_` into the flags bitfield
    uiInternalFlags looks racy to me (though I think really the old code
    was theoretically wrong already, the refactor is probably just making
    that worse): Bits in this field are updated with BITMASK_SET() /
    BITMASK_UNSET(), which are non-atomic; and the callers of these
    helpers don't seem to be using any other locking to avoid concurrent
    writes to this field. In theory, this kind of concurrency is
    forbidden; in practice, the consequence is that updates can get lost
    when two threads try to update different parts of the bitfield, like
    so:
    
    thread A: reads old value of uiInternalFlags (0) thread B: reads old
    value of uiInternalFlags (0) thread A: writes observed value ORed with
    PMR_FLAG_INTERNAL_NO_LAYOUT_CHANGE
    thread B: writes observed value ORed with PMR_FLAG_INTERNAL_DEFER_FREE
    
    causing the final value of uiInternalFlags to be
    PMR_FLAG_INTERNAL_DEFER_FREE, with thread A's write having been lost.
    
    There are two ways around this: Either ensure all places that access
    this flags field hold the same lock, or use atomic RMW operations
    (such as set_bit() on Linux, though note that that's a relaxed atomic
    operation and I haven't checked whether any of your uses of these
    flags would require stronger memory ordering).
    
    
    Marked as fixed.
    The fix is public at https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b61103dc9ad5aa2ae3229dfa64066ed9414abe2e%5E%21/ .
    
    F.5 has been addressed by changing the recheck-after-relock in _PmrZombieCleanup() to use `psNode == dllist_get_next_node(&psCleanupItem->sZombieList)` instead of `PMR_IsZombie(psPMR)`.
    
    F.9 has been addressed with the introduction of _IntFlagSet() / _IntFlagClr() / _IntFlagIsSet() for atomic bitops on the ->uiInternalFlags. (It looks like PMRPinPMR/PMRUnpinPMR/PMR_IsUnpinned weren't changed, but at least in ChromeOS, PMRPinPMR/PMRUnpinPMR seem to be dead code.)
    
    
    Related CVE Number: CVE-2024-40670
    
    Credit: Jann Horn