From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/vc4: Protect madv read in vc4_gem_object_mmap() with madv_lock Date: Tue, 31 Mar 2026 17:02:00 +1000 Message-ID: In-Reply-To: <20260330-vc4-misc-fixes-v1-4-92defc940a29@igalia.com> References: <20260330-vc4-misc-fixes-v1-0-92defc940a29@igalia.com> <20260330-vc4-misc-fixes-v1-4-92defc940a29@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Verdict: Mostly good, minor observation.** The fix adds `mutex_lock(&bo->madv_lock)` around the `bo->madv` check to pr= event a data race with `DRM_IOCTL_VC4_GEM_MADVISE`. ```c +mutex_lock(&bo->madv_lock); if (bo->madv !=3D VC4_MADV_WILLNEED) { ... + mutex_unlock(&bo->madv_lock); return -EINVAL; } +mutex_unlock(&bo->madv_lock); =20 return drm_gem_dma_mmap(&bo->base, vma); ``` There is a TOCTOU window here: `madv` could change between dropping the loc= k and calling `drm_gem_dma_mmap()`. However, this is a pre-existing design = pattern in the driver =E2=80=94 the same TOCTOU exists in other madv-checki= ng paths, and the comment in `vc4_save_hang_state` acknowledges that BO con= sistency "cannot be guaranteed" when userspace acts concurrently. The fix i= s consistent with how the rest of the driver handles this, and closes the d= ata race on the read itself, which is the actual bug (unsynchronized access= to a shared variable). No blocking issues. --- --- Generated by Claude Code Patch Reviewer