⬢ DragonFlyBSD Kernel Audit
DF-3656 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/sys/dev/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1862,10 +1862,12 @@
 		aconnector = kzalloc(sizeof(*aconnector), GFP_KERNEL);
 		if (!aconnector)
 			goto fail;
+		aconnector->connector_id = -1;
 
 		aencoder = kzalloc(sizeof(*aencoder), GFP_KERNEL);
 		if (!aencoder)
 			goto fail;
+		aencoder->encoder_id = -1;
 
 		if (amdgpu_dm_encoder_init(dm->ddev, aencoder, i)) {
 			DRM_ERROR("KMS: Failed to initialize encoder\n");
@@ -1935,10 +1937,22 @@
 
 	return 0;
 fail:
-	kfree(aencoder);
-	kfree(aconnector);
+	/*
+	 * Only free the current iteration's aencoder/aconnector when drm
+	 * core does not own them yet (early failure).  Objects that were
+	 * already registered (connector/encoder/plane lists) are released
+	 * by drm_mode_config_cleanup() through amdgpu_dm_destroy_drm_device()
+	 * in the caller's error unwind - kfree()ing them here would leave
+	 * dangling pointers in the mode_config lists (use-after-free).
+	 */
+	if (aencoder && aencoder->encoder_id == -1)
+		kfree(aencoder);
+	if (aconnector && aconnector->connector_id == -1)
+		kfree(aconnector);
 	for (i = 0; i < dm->dc->caps.max_planes; i++)
-		kfree(mode_info->planes[i]);
+		if (mode_info->planes[i] &&
+		    !mode_info->planes[i]->base.dev)
+			kfree(mode_info->planes[i]);
 	return -EINVAL;
 }
 
@@ -4065,6 +4079,7 @@
 	struct dc *dc = dm->dc;
 	struct dc_link *link = dc_get_link_at_index(dc, link_index);
 	struct amdgpu_i2c_adapter *i2c;
+	bool i2c_registered = false;
 
 	link->priv = aconnector;
 
@@ -4083,6 +4098,7 @@
 		DRM_ERROR("Failed to register hw i2c %d\n", link->link_index);
 		goto out_free;
 	}
+	i2c_registered = true;
 
 	connector_type = to_drm_connector_type(link->connector_signal);
 
@@ -4130,6 +4146,10 @@
 
 out_free:
 	if (res) {
+		/* de-register before free: the adapter was added to the
+		 * i2c core and is referenced from there (UAF otherwise) */
+		if (i2c_registered)
+			i2c_del_adapter(&i2c->base);
 		kfree(i2c);
 		aconnector->i2c = NULL;
 	}