diff --git a/.github/workflows/drm-capture.yml b/.github/workflows/drm-capture.yml index fa4d47b9e..372acf2bf 100644 --- a/.github/workflows/drm-capture.yml +++ b/.github/workflows/drm-capture.yml @@ -278,7 +278,15 @@ jobs: shell: bash run: | cd $(dirname $(dirname $(which flutter))) - [[ "3.24.5" == ${{ env.FLUTTER_VERSION }} ]] && git apply ${{ github.workspace }}/.github/patches/flutter_3.24.4_dropdown_menu_enableFilter.diff + # `[[ ... ]] && cmd` as the last line makes the STEP fail once FLUTTER_VERSION moves off + # the pinned value, because the failed test becomes the script's exit status. An explicit + # if/else skips instead. Reading the values from the environment rather than interpolating + # ${{ }} into the script also keeps this off zizmor's template-injection list. + if [[ "$FLUTTER_VERSION" == "3.24.5" ]]; then + git apply "$GITHUB_WORKSPACE/.github/patches/flutter_3.24.4_dropdown_menu_enableFilter.diff" + else + echo "::notice::flutter $FLUTTER_VERSION is not 3.24.5; skipping the dropdown patch" + fi - name: Build the unattended-wayland deb shell: bash diff --git a/build.py b/build.py index 14a258b3e..1d3a5158d 100755 --- a/build.py +++ b/build.py @@ -409,7 +409,14 @@ def build_libdrmtap_so(): # DRMTAP_PREBUILT_DIR explicitly names the artifact source, so honor it strictly: fail # (rather than silently falling back to a source build) if it holds no single real .so. prebuilt = glob.glob(os.path.join(prebuilt_dir, 'libdrmtap.so.0.*')) - return _single_real_so(prebuilt, f'DRMTAP_PREBUILT_DIR={prebuilt_dir}') + so = _single_real_so(prebuilt, f'DRMTAP_PREBUILT_DIR={prebuilt_dir}') + # Check the stub case HERE too, not only on the source path below. This is the widest + # override of the three -- no fetch, no sha verification, an object built by something this + # script cannot see -- so it is the likeliest to hand over a CPU-only build, and skipping the + # assertion on exactly this path would leave the check guarding only the case that was + # already trustworthy. + _assert_so_has_egl(so) + return so # Fetch the pinned source if it is not already present. third_party/libdrmtap is not a submodule # anymore; it is git-ignored. The commit is fetched BY SHA rather than by cloning a branch: # `clone --depth 1 --branch main` only ever fetches the tip, so the moment upstream pushes to diff --git a/libs/scrap/src/common/drm_render.rs b/libs/scrap/src/common/drm_render.rs index 1218de1ba..e5f89a602 100644 --- a/libs/scrap/src/common/drm_render.rs +++ b/libs/scrap/src/common/drm_render.rs @@ -116,6 +116,43 @@ impl RenderConverter { desc: &mut drmtap_dmabuf_desc, received_fd: RawFd, ) -> io::Result<(&[u8], u32, u32, Pixfmt)> { + // Bound the INCOMING descriptor, not just the frame libdrmtap hands back. Every field here + // was deserialized from the `_drm` wire, and while the producer is authenticated as root + // (`connect_drm` refuses a non-root peer) and libdrmtap has validated the fd against the + // descriptor since 0.4.12, this side already bounds the export descriptor symmetrically -- + // the two halves of the split should agree about what they will touch before the C sees it, + // not after. + // + // offsets/pitches are the interesting ones: they address plane ranges inside the dma-buf, so + // a malformed pair is what would reach past the buffer. Bound each populated plane's extent + // the same way the frame path is bounded. + { + let (w, h) = (desc.width, desc.height); + if w == 0 || h == 0 || w > MAX_DIM || h > MAX_DIM { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("drm: refusing a dma-buf descriptor with geometry {w}x{h}"), + )); + } + let planes = desc.num_planes.clamp(1, 4) as usize; + for p in 0..planes { + let extent = (desc.pitches[p] as usize) + .checked_mul(h as usize) + .and_then(|rows| rows.checked_add(desc.offsets[p] as usize)); + match extent { + Some(end) if end <= MAX_FRAME_BYTES => {} + other => { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "drm: refusing dma-buf plane {p} (offset {} pitch {} over {h} rows -> {other:?}, cap {MAX_FRAME_BYTES})", + desc.offsets[p], desc.pitches[p] + ), + )); + } + } + } + } // Always bound: a libdrmtap without the split convert symbols never loads. let convert_dmabuf = self.lib.convert_dmabuf; // Overwrite the descriptor's fd with the one THIS process received (split_capture.c diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index 4b1ddfa15..9801bdfcc 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -797,10 +797,15 @@ async fn recv_thread( // runs ONE thread and the final layout wins. Not ordered against frame delivery. 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); + // Take ownership of the flag BEFORE the spawn and move it into the closure. + // Constructing the guard inside the closure only covers paths where the closure + // actually runs: `thread::spawn` panics on EAGAIN, and that happens after the + // swap above, so no guard would ever exist and the flag would stay set for the + // process lifetime. Same mistake, same shape, as the two flags before it. + let mut busy = UinputRefreshGuard(true); + let spawned = std::thread::Builder::new() + .name("drm-uinput-refresh".into()) + .spawn(move || { let rt = match tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -835,6 +840,12 @@ async fn recv_thread( } } }); + if let Err(err) = spawned { + // The closure never ran, so it was dropped along with the guard it owned, + // and the guard released the slot: the next topology change retries the + // spawn instead of being locked out for the process lifetime. + log::error!("drm: could not spawn the uinput refresh worker: {err}"); + } } } _ => {} // ignore any unexpected control message