From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/amdgpu/amdgpu_connectors: remove
 amdgpu_connector_free_edid
Date: Fri, 13 Feb 2026 16:24:45 +1000
Message-ID: <review-patch2-20260212222029.15777-3-jpeisach@ubuntu.com>
In-Reply-To: <20260212222029.15777-3-jpeisach@ubuntu.com>
References: <20260212222029.15777-1-jpeisach@ubuntu.com>
 <20260212222029.15777-3-jpeisach@ubuntu.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

This patch removes the wrapper function and calls `drm_edid_free()` directly =
at all call sites. While this seems like a straightforward cleanup, it introd=
uces a critical resource management bug.

> @@ -297,14 +297,6 @@ static void amdgpu_connector_get_edid(struct drm_conne=
ctor *connector)
>  	}
>  }
> =20
> -static void amdgpu_connector_free_edid(struct drm_connector *connector)
> -{
> -	struct amdgpu_connector *amdgpu_connector =3D to_amdgpu_connector(connect=
or);
> -
> -	kfree(amdgpu_connector->edid);
> -	amdgpu_connector->edid =3D NULL;
> -}

The removed function performs two critical operations: freeing the EDID and s=
etting the pointer to NULL. The replacement code below only does the first op=
eration.

> @@ -873,7 +865,7 @@ amdgpu_connector_vga_detect(struct drm_connector *conne=
ctor, bool force)
>  	if (dret) {
>  		amdgpu_connector->detected_by_load =3D false;
> -		amdgpu_connector_free_edid(connector);
> +		drm_edid_free(amdgpu_connector->edid);
>  		amdgpu_connector_get_edid(connector);

This is the first problematic location. After freeing `amdgpu_connector->edid=
`, the pointer is not set to NULL. Then `amdgpu_connector_get_edid()` is call=
ed. Looking at line 261 of that function:

```c
if (amdgpu_connector->edid)
    return;
```

This early-return check now sees a dangling pointer instead of NULL, so the f=
unction returns immediately without fetching new EDID data. This breaks the i=
ntended behavior of refreshing the EDID after freeing the old one. The same i=
ssue appears at lines 1048-1049 in the DVI detect function.

> @@ -883,7 +881,7 @@ amdgpu_connector_vga_detect(struct drm_connector *conne=
ctor, bool force)
>  		 */
>  		if (amdgpu_connector->use_digital && amdgpu_connector->shared_ddc) {
> -			amdgpu_connector_free_edid(connector);
> +			drm_edid_free(amdgpu_connector->edid);
>  			ret =3D connector_status_disconnected;
>  		} else {
>  			ret =3D connector_status_connected;

After freeing the EDID here, if the code path later reaches `amdgpu_connector=
_destroy()` (line 749) or returns through another path that accesses `amdgpu_=
connector->edid`, you'll have use-after-free or double-free bugs. The same is=
sue occurs at lines 1064, 979, and 1412.

The fix is straightforward: after each `drm_edid_free(amdgpu_connector->edid)=
` call, add `amdgpu_connector->edid =3D NULL;`. Alternatively, create a helpe=
r function that does both operations atomically, or modify the locations to u=
se a pattern like:

```c
const struct drm_edid *old_edid =3D amdgpu_connector->edid;
amdgpu_connector->edid =3D NULL;
drm_edid_free(old_edid);
```

The only location where setting NULL is not needed is in `amdgpu_connector_de=
stroy()` (line 749) since the entire connector structure is being freed immed=
iately after.

---
Generated by Claude Code Patch Reviewer