[PATCH] drm/client: Do not destroy NULL modes

[PATCH] drm/client: Do not destroy NULL modes

[Jonathan Cavitt]

'modes' in drm_client_modeset_probe may fail to kcalloc.  If this
occurs, we jump to 'out', calling modes_destroy on it, which
dereferences it.  This may result in a NULL pointer dereference in the
error case.  Prevent that.

Fixes: 3039cc0c0653 ("drm/client: Make copies of modes")
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_client_modeset.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 262b1b8773c5..bb49b8361271 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -930,7 +930,8 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
 	mutex_unlock(&client->modeset_mutex);
 out:
 	kfree(crtcs);
-	modes_destroy(dev, modes, connector_count);
+	if (modes)
+		modes_destroy(dev, modes, connector_count);
 	kfree(modes);
 	kfree(offsets);
 	kfree(enabled);
-- 
2.43.0

Re: [PATCH] drm/client: Do not destroy NULL modes

[Ville Syrjälä]

On Tue, Feb 24, 2026 at 10:12:28PM +0000, Jonathan Cavitt wrote:
> 'modes' in drm_client_modeset_probe may fail to kcalloc.  If this
> occurs, we jump to 'out', calling modes_destroy on it, which
> dereferences it.  This may result in a NULL pointer dereference in the
> error case.  Prevent that.
> 
> Fixes: 3039cc0c0653 ("drm/client: Make copies of modes")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks. Pushed to drm-misc-fixes.

> ---
>  drivers/gpu/drm/drm_client_modeset.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 262b1b8773c5..bb49b8361271 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -930,7 +930,8 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>  	mutex_unlock(&client->modeset_mutex);
>  out:
>  	kfree(crtcs);
> -	modes_destroy(dev, modes, connector_count);
> +	if (modes)
> +		modes_destroy(dev, modes, connector_count);
>  	kfree(modes);
>  	kfree(offsets);
>  	kfree(enabled);
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel

Claude review: drm/client: Do not destroy NULL modes

[Claude Code Review Bot]

Overall Series Review

Subject: drm/client: Do not destroy NULL modes
Author: Jonathan Cavitt <jonathan.cavitt@intel.com>
Patches: 2
Reviewed: 2026-02-27T14:35:50.430766

---

This is a single-patch submission fixing a potential NULL pointer dereference in `drm_client_modeset_probe()`. The fix is correct but could be slightly improved. The issue is real: when any of `crtcs`, `modes`, `offsets`, or `enabled` fail to allocate, the code jumps to `out:` and calls `modes_destroy(dev, modes, connector_count)`, which iterates `modes[]` unconditionally — dereferencing a NULL pointer if `modes` itself failed to allocate.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/client: Do not destroy NULL modes

[Claude Code Review Bot]

Patch Review

**Bug analysis:** Correct. Looking at lines 856–862 of `drm_client_modeset.c`:

```c
crtcs = kzalloc_objs(*crtcs, connector_count);
modes = kzalloc_objs(*modes, connector_count);
offsets = kzalloc_objs(*offsets, connector_count);
enabled = kzalloc_objs(bool, connector_count);
if (!crtcs || !modes || !enabled || !offsets) {
    ret = -ENOMEM;
    goto out;
}
```

If `crtcs` succeeds but `modes` fails, we jump to `out:` where `modes_destroy()` is called with `modes == NULL`. Inside `modes_destroy()`:

```c
static void modes_destroy(struct drm_device *dev,
                          const struct drm_display_mode *modes[],
                          int count)
{
    int i;

    for (i = 0; i < count; i++)
        mode_replace(dev, &modes[i], NULL);
}
```

The `modes[i]` dereference with `modes == NULL` is indeed a NULL pointer dereference. The fix is correct.

**Minor suggestion:** `kfree(NULL)` is safe in the kernel, and `kfree(modes)` already follows the `modes_destroy()` call. The guard added is:

```c
if (modes)
    modes_destroy(dev, modes, connector_count);
```

This is fine, but an alternative would be to add the NULL check inside `modes_destroy()` itself, which would make it defensive at the source rather than requiring every caller to remember the guard. That said, there is currently only one caller of `modes_destroy()`, so this approach is acceptable.

**Nit on the commit message:** The commit message says `modes` "may fail to kcalloc", but the actual allocation macro used is `kzalloc_objs` (which wraps `kzalloc`), not `kcalloc`. This is a very minor inaccuracy — consider saying "may fail to allocate" instead.

**Verdict:** The fix is correct and the Fixes tag references the right commit. The patch is ready to merge with at most a commit message wording tweak.

**Reviewed-by worthy:** Yes, with the minor commit message nit noted above.

---
Generated by Claude Code Patch Reviewer