diff --git a/.github/workflows/drm-capture.yml b/.github/workflows/drm-capture.yml index 5e7ac112d..e25d0e292 100644 --- a/.github/workflows/drm-capture.yml +++ b/.github/workflows/drm-capture.yml @@ -63,6 +63,7 @@ jobs: drm-tests: name: drm unit tests (linux) runs-on: ubuntu-24.04 + timeout-minutes: 60 steps: - name: Free Disk Space (Ubuntu) uses: jlumbroso/free-disk-space@54081f138730dfa15788a46383842cd2f914a1be # v1.3.1 @@ -131,6 +132,7 @@ jobs: libdrmtap: name: libdrmtap pin, build and .so contract runs-on: ubuntu-24.04 + timeout-minutes: 60 steps: - name: Checkout source code uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 @@ -199,6 +201,7 @@ jobs: name: unattended-wayland deb (verification build) needs: generate-bridge runs-on: ubuntu-24.04 + timeout-minutes: 60 steps: - name: Free Disk Space (Ubuntu) uses: jlumbroso/free-disk-space@54081f138730dfa15788a46383842cd2f914a1be # v1.3.1 diff --git a/libs/scrap/src/common/drm_reader.rs b/libs/scrap/src/common/drm_reader.rs index e6ed9a5b1..3de240af1 100644 --- a/libs/scrap/src/common/drm_reader.rs +++ b/libs/scrap/src/common/drm_reader.rs @@ -256,6 +256,24 @@ impl DrmReader { )); } }; + // Bound the SOURCE extent too, not just the destination. The row loop below reads up to + // (h-1)*stride + w*4, so a large stride reads far past the mapping however small the + // destination is, and `y * stride` can overflow usize on the way. drm_render::convert + // bounds stride*h the same way; the two halves of the split must agree about what they + // are willing to touch, or the privileged half is the weaker one. + match stride.checked_mul(h) { + Some(sz) if sz > 0 && sz <= MAX_FRAME_BYTES => {} + other => { + log::warn!( + "DRM scanout stride {stride} x {h} rows is out of range ({other:?} bytes); falling back" + ); + (self.lib.frame_release)(self.ctx, &mut frame); + return Err(io::Error::new( + io::ErrorKind::Other, + "DRM scanout stride out of range", + )); + } + } if self.buf.len() != frame_size { self.buf.resize(frame_size, 0); } diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index f9bb0b4b0..4b1ddfa15 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -798,6 +798,9 @@ async fn recv_thread( UINPUT_REFRESH_GEN.fetch_add(1, Ordering::AcqRel); if !UINPUT_REFRESH_BUSY.swap(true, Ordering::AcqRel) { std::thread::spawn(|| { + // We hold the flag from the swap above; the guard hands it back on every + // exit from here, including a panic inside the refresh below. + let mut busy = UinputRefreshGuard(true); let rt = match tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -810,8 +813,7 @@ async fn recv_thread( log::warn!( "drm: uinput refresh worker could not build a runtime: {err}" ); - UINPUT_REFRESH_BUSY.store(false, Ordering::Release); - return; + return; // the guard hands the slot back } }; let mut served = 0u64; @@ -824,11 +826,11 @@ async fn recv_thread( } // Caught up: release, then re-check for a request that raced in after our // load but before the release, taking the worker role back if so. - UINPUT_REFRESH_BUSY.store(false, Ordering::Release); + busy.release(); if UINPUT_REFRESH_GEN.load(Ordering::Acquire) == served { break; } - if UINPUT_REFRESH_BUSY.swap(true, Ordering::AcqRel) { + if !busy.retake() { break; // another handler already started a fresh worker } } @@ -1031,6 +1033,37 @@ impl Drop for ProbeInFlightGuard { } } +/// Ownership of `UINPUT_REFRESH_BUSY`, released on every exit including an unwind. Same job as +/// `ProbeInFlightGuard`, but this flag is deliberately handed back and re-taken mid-loop (the +/// caught-up/re-check dance), so it has to track whether we still hold it: releasing on drop +/// unconditionally would clear a flag a REPLACEMENT worker owns. +/// +/// Without this the worker body could leave it latched true forever -- it locks several process-wide +/// mutexes and does a Wayland roundtrip -- and every later hotplug would then skip the spawn and +/// never reapply the uinput range, which is the stale-range/wrong-output symptom +/// `wayland::update_uinput_resolution` is written to prevent. +struct UinputRefreshGuard(bool); +impl UinputRefreshGuard { + /// Hand the flag back, if we are the ones holding it. + fn release(&mut self) { + if self.0 { + self.0 = false; + UINPUT_REFRESH_BUSY.store(false, Ordering::Release); + } + } + /// Take the worker role back. False when someone else already did, in which case we are NOT the + /// owner and must not release it on the way out. + fn retake(&mut self) -> bool { + self.0 = !UINPUT_REFRESH_BUSY.swap(true, Ordering::AcqRel); + self.0 + } +} +impl Drop for UinputRefreshGuard { + fn drop(&mut self) { + self.release(); + } +} + /// Whether the root service offers DRM/KMS capture. The positive result and a definitive negative /// (connected, but no displays) are cached; a transient probe error stays `Unknown` for a few /// retries. Normally the cache is warmed at `--server` startup (`warm_availability`), so the first