[PATCH] drm/modeset_lock: add NULL check for ctx before WARN_ON

[PATCH] drm/modeset_lock: add NULL check for ctx before WARN_ON

[george.d.sworo]

From: George D Sworo <george.d.sworo@intel.com>

modeset_lock() and drm_modeset_drop_locks() do not validate
the ctx pointer before dereferencing it in WARN_ON(ctx->contended),
which can lead to a NULL pointer dereference if ctx is NULL.

Add a NULL check to prevent this.

Signed-off-by: George D Sworo <george.d.sworo@intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index beb91a13a312..2052bb9bb9e5 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -295,7 +295,7 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
 {
 	int ret;
 
-	if (WARN_ON(ctx->contended))
+	if (ctx && WARN_ON(ctx->contended))
 		__drm_stack_depot_print(ctx->stack_depot);
 
 	if (ctx->trylock_only) {
-- 
2.34.1

[PATCH v2 0/2] drm/i915/modeset: fix NULL/ctx handling in lock paths

[george.d.sworo]

From: George D Sworo <george.d.sworo@intel.com>

Hi,

This series addresses lock-context robustness seen in HDCP/MST paths,
where modeset locking can be reached with invalid or stale acquire_ctx
state and trigger WARN/Oops in modeset_lock().

Patch 1 keeps the original fix from v1.
Patch 2 adds a defensive fix for the additional ctx handling issue.

Changes in v2:
- Added patch 2 to handle NULL ctx defensively in modeset lock helper
  path.
- Kept patch 1 from v1 (no functional change) for complete series
  resend.
- Updated commit messages for clearer rationale and call-path context.
- Linked previous posting:
  https://patchwork.freedesktop.org/patch/721791/

Patch overview:
  drm/modeset_lock: add NULL check for ctx before WARN_ON
  drm/modeset: harden modeset_lock() against NULL ctx

 drivers/gpu/drm/drm_modeset_lock.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH v2 1/2] drm/modeset_lock: add NULL check for ctx before WARN_ON

[george.d.sworo]

From: George D Sworo <george.d.sworo@intel.com>

modeset_lock() and drm_modeset_drop_locks() do not validate
the ctx pointer before dereferencing it in WARN_ON(ctx->contended),
which can lead to a NULL pointer dereference if ctx is NULL.

Add a NULL check to prevent this.

Signed-off-by: George D Sworo <george.d.sworo@intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index beb91a13a312..2052bb9bb9e5 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -295,7 +295,7 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
 {
 	int ret;
 
-	if (WARN_ON(ctx->contended))
+	if (ctx && WARN_ON(ctx->contended))
 		__drm_stack_depot_print(ctx->stack_depot);
 
 	if (ctx->trylock_only) {
-- 
2.34.1

[PATCH v2 2/2] drm/modeset: harden modeset_lock() against NULL ctx

[george.d.sworo]

From: George D Sworo <george.d.sworo@intel.com>

modeset_lock() dereferences ctx unconditionally. Add a defensive NULL
guard to avoid NULL dereference if a buggy internal caller passes a NULL
acquire context.

For NULL ctx, fall back to plain ww_mutex locking semantics:
- interruptible path uses ww_mutex_lock_interruptible(..., NULL)
- non-interruptible path uses ww_mutex_lock(..., NULL)

This keeps wait behavior consistent with the helper arguments.

Signed-off-by: George D Sworo <george.d.sworo@intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 2052bb9bb9e5..5bee424805c3 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -294,6 +294,18 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
 		bool interruptible, bool slow)
 {
 	int ret;
+	/*
+	 * Defensive fallback: this helper is expected to be called with a
+	 * valid acquire context, but if a NULL ctx slips through, preserve
+	 * the lock wait semantics and avoid NULL dereference.
+	 */
+	if (unlikely(!ctx)) {
+		if (interruptible)
+			return ww_mutex_lock_interruptible(&lock->mutex, NULL);
+
+		ww_mutex_lock(&lock->mutex, NULL);
+		return 0;
+	}
 
 	if (ctx && WARN_ON(ctx->contended))
 		__drm_stack_depot_print(ctx->stack_depot);
-- 
2.34.1

Re: [PATCH v2 1/2] drm/modeset_lock: add NULL check for ctx before WARN_ON

[Ville Syrjälä]

On Tue, Apr 28, 2026 at 11:04:30PM -0700, george.d.sworo@intel.com wrote:
> From: George D Sworo <george.d.sworo@intel.com>
> 
> modeset_lock() and drm_modeset_drop_locks() do not validate
> the ctx pointer before dereferencing it in WARN_ON(ctx->contended),
> which can lead to a NULL pointer dereference if ctx is NULL.
> 
> Add a NULL check to prevent this.

Why are you trying to pass garbage into the function?

> 
> Signed-off-by: George D Sworo <george.d.sworo@intel.com>
> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> index beb91a13a312..2052bb9bb9e5 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -295,7 +295,7 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
>  {
>  	int ret;
>  
> -	if (WARN_ON(ctx->contended))
> +	if (ctx && WARN_ON(ctx->contended))
>  		__drm_stack_depot_print(ctx->stack_depot);
>  
>  	if (ctx->trylock_only) {
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 1/2] drm/modeset_lock: add NULL check for ctx before WARN_ON

[Sworo, George D]

Thanks Ville for the review!
Fair point. Ideally, we shouldn't be calling modeset_lock() with NULL
ctx but during a recent debug we uncovered an issue where the system
kept printing the calltrace and I figured this was one instance where
ctx is being dereferenced without a guard

03-26 11:33:03.261     0     0 F BUG     : kernel NULL pointer
dereference, address: 0000000000000069
03-26 11:33:03.261     0     0 F #PF     : supervisor read access in
kernel mode
03-26 11:33:03.261     0     0 F #PF     : error_code(0x0000) - not-
present page
03-26 11:33:03.261     0     0 I         : PGD 0 P4D 0
03-26 11:33:03.261     0     0 W Oops    : Oops: 0000 [#1] PREEMPT SMP
NOPTI
03-26 11:33:03.261     0     0 W Tainted : [U]=USER, [W]=WARN,
[O]=OOT_MODULE
03-26 11:33:03.261     0     0 W Workqueue: i915_flip
intel_atomic_commit_work [xe]
03-26 11:33:03.262     0     0 W RIP     : 0010:modeset_lock+0x74/0xd0



On Wed, 2026-04-29 at 14:09 +0300, Ville Syrjälä wrote:
> On Tue, Apr 28, 2026 at 11:04:30PM -0700,
> george.d.sworo@intel.com wrote:
> > From: George D Sworo <george.d.sworo@intel.com>
> > 
> > modeset_lock() and drm_modeset_drop_locks() do not validate
> > the ctx pointer before dereferencing it in WARN_ON(ctx->contended),
> > which can lead to a NULL pointer dereference if ctx is NULL.
> > 
> > Add a NULL check to prevent this.
> 
> Why are you trying to pass garbage into the function?
> 
> > 
> > Signed-off-by: George D Sworo <george.d.sworo@intel.com>
> > ---
> >  drivers/gpu/drm/drm_modeset_lock.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_modeset_lock.c
> > b/drivers/gpu/drm/drm_modeset_lock.c
> > index beb91a13a312..2052bb9bb9e5 100644
> > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > @@ -295,7 +295,7 @@ static inline int modeset_lock(struct
> > drm_modeset_lock *lock,
> >  {
> >  	int ret;
> >  
> > -	if (WARN_ON(ctx->contended))
> > +	if (ctx && WARN_ON(ctx->contended))
> >  		__drm_stack_depot_print(ctx->stack_depot);
> >  
> >  	if (ctx->trylock_only) {
> > -- 
> > 2.34.1
> 

Claude review: drm/i915/modeset: fix NULL/ctx handling in lock paths

[Claude Code Review Bot]

Overall Series Review

Subject: drm/i915/modeset: fix NULL/ctx handling in lock paths
Author: george.d.sworo@intel.com
Patches: 6
Reviewed: 2026-05-05T11:50:03.943890

---

**NAK.** This series adds defensive NULL checks for `ctx` inside the static helper `modeset_lock()`, but this function **can never be called with a NULL `ctx`** in the existing code. The two callers are:

1. `drm_modeset_lock()` (line 394) — which already handles `ctx == NULL` at line 397 and only calls `modeset_lock()` when `ctx` is non-NULL.
2. `drm_modeset_backoff()` (line 360) — which dereferences `ctx->contended` before calling `modeset_lock()`, so `ctx` is necessarily non-NULL.

The cover letter claims this is "seen in HDCP/MST paths" but provides no crash log, stack trace, or reproducer. If there is genuinely a code path reaching this function with a NULL context, the correct fix is to fix the broken caller, not to silently paper over it in a core locking primitive. Adding a silent fallback to plain `ww_mutex_lock(..., NULL)` would bypass deadlock detection and lock tracking (`ctx->locked` list), which would mask real bugs and cause worse problems downstream (e.g., `drm_modeset_drop_locks()` wouldn't know about the lock).

The commit messages also claim `drm_modeset_drop_locks()` has the same problem, but neither patch touches that function.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/modeset_lock: add NULL check for ctx before WARN_ON

[Claude Code Review Bot]

Patch Review

**NAK.**

The change:
```c
-	if (WARN_ON(ctx->contended))
+	if (ctx && WARN_ON(ctx->contended))
```

**Problem 1: The guard is unnecessary.** As discussed above, `modeset_lock()` is a static helper only called from `drm_modeset_lock()` (which already NULL-checks `ctx` before calling it) and `drm_modeset_backoff()` (which dereferences `ctx` before calling it). There is no path where `ctx` is NULL here.

**Problem 2: The patch is incomplete even on its own terms.** It guards the `ctx->contended` dereference on line 298, but the very next line that uses `ctx` is:
```c
	if (ctx->trylock_only) {
```
This dereference at line 301 is left completely unguarded. If `ctx` truly could be NULL, this patch would still crash two lines later. Patch 1 in isolation does not produce a correct result.

**Problem 3: Inaccurate commit message.** The commit message says "modeset_lock() and drm_modeset_drop_locks() do not validate the ctx pointer" but neither this patch nor patch 2 modifies `drm_modeset_drop_locks()`.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/modeset: harden modeset_lock() against NULL ctx

[Claude Code Review Bot]

Patch Review

**NAK.**

The change adds an early-return path for NULL `ctx`:
```c
+	if (unlikely(!ctx)) {
+		if (interruptible)
+			return ww_mutex_lock_interruptible(&lock->mutex, NULL);
+
+		ww_mutex_lock(&lock->mutex, NULL);
+		return 0;
+	}
```

**Problem 1: Still fixing a non-existent bug.** As above, `ctx` is never NULL here.

**Problem 2: The fallback silently breaks locking invariants.** If this code were ever reached, the lock would be acquired via plain `ww_mutex_lock(..., NULL)` but would **not** be added to `ctx->locked`. That means `drm_modeset_drop_locks()` would not release it, causing a deadlock. The whole point of the `modeset_lock()` helper is to integrate with the acquire-context tracking (lines 318-331); bypassing that tracking is worse than crashing.

**Problem 3: Excessive comment.** The 4-line comment block explains what the code does, which is already self-evident from the `if (!ctx)` check.

**Problem 4: Makes patch 1 redundant.** With this early return in place, the `ctx &&` guard added in patch 1 can never trigger (we already returned if `ctx` is NULL). The two patches together are incoherent — patch 2 makes patch 1's change dead code.

**Recommendation:** If the author is genuinely hitting a NULL `ctx` crash, they should provide a full stack trace and fix the actual caller that is passing NULL. The existing `drm_modeset_lock()` API (line 394) already correctly handles NULL `ctx` — the bug would be in whatever is calling an internal path without going through the public API.

---
Generated by Claude Code Patch Reviewer