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/atomic: Allocate atomic_state at the beginning of atomic_ioctl Date: Wed, 01 Apr 2026 07:55:27 +1000 Message-ID: In-Reply-To: <20260331-atomic-v11-4-6a1df7ec5af8@intel.com> References: <20260331-atomic-v11-0-6a1df7ec5af8@intel.com> <20260331-atomic-v11-4-6a1df7ec5af8@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This moves `drm_atomic_state_alloc()` and `drm_modeset_acquire_init()` before the early validation checks, converting early `return -EINVAL` to `goto out`. This means every trivially-invalid call now allocates and frees an atomic state, acquires/releases modeset locks, etc. That's unnecessary overhead for simple validation failures. The original code was structured to fail fast cheaply. The justification is to have `state->error_code` available, but for these trivial "invalid flag" / "reserved field set" cases, the error is already clear from the errno. Consider only allocating early when the error reporting pointer (`arg->reserved`) is actually provided. **The `arg->reserved` check is removed** from the validation: ```c - if (arg->reserved) { - drm_dbg_atomic(dev, "commit failed: reserved field set\n"); - return -EINVAL; - } ``` This is the mechanism that ensures forward/backward compatibility. Removing it without a versioning scheme means old kernels that don't understand the new meaning of `reserved` will reject the call, which is fine, but old userspace that sets `reserved` to garbage will now have `copy_to_user` write to a random address. The capability check in patch 7 partially addresses this (new userspace should check the cap first), but the kernel should still validate the pointer somehow. --- --- Generated by Claude Code Patch Reviewer