Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   Donald Robson <donald.robson@imgtec.com>
To:     <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
CC:     <frank.binns@imgtec.com>, <donald.robson@imgtec.com>,
        <matt.coster@imgtec.com>, <maarten.lankhorst@linux.intel.com>,
        <mripard@kernel.org>, <tzimmermann@suse.de>, <airlied@gmail.com>,
        <daniel@ffwll.ch>, Dan Carpenter <dan.carpenter@linaro.org>
Subject: [PATCH 2/2] drm/imagination: Fix error path in pvr_vm_create_context
Date:   Wed, 13 Dec 2023 14:44:31 +0000
Message-Id: <20231213144431.94956-2-donald.robson@imgtec.com>
In-Reply-To: <20231213144431.94956-1-donald.robson@imgtec.com>
References: <20231213144431.94956-1-donald.robson@imgtec.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N0pnYTU0WWd4ZWpRS2NrMVkvN2JOblRrTURQeUZIdTFVV2NLVUhSRHdWd1dp?=
 =?utf-8?B?RnBGc203NENDcTYwazFmZCtZbDhJYUdBQU9GdFgxRUtHUjVvcCtxLzFKRmhp?=
 =?utf-8?B?N0owVmxEN0Y3Q3B5ZnIrZHk1bStnYkxTbTBXUTNNcGhrMjdCRlpYN2NlWXI1?=
 =?utf-8?B?K2lFUW9WeTRlV1FQc3lMZ3IvdnJkKytYcGdpTDc0UVFOc1draEMza24rQ1BF?=
 =?utf-8?B?dHptWCtMeEt5MGtRWEtVSk9GcXJBa0RGMjRteEhEQVV5U01Vc1ZwZEpSd1Rz?=
 =?utf-8?B?ZGNXVEEwa05ndXRYaGNxaVJrRTBFYUJNcjFlbEh6R2owalB1SEI2TWM0Wm82?=
 =?utf-8?B?V0lzQSt6c0lPVE4xNlZjQ2NTbGhwekIrRGIrWHZ1Um9mSXR1OWN1bERHUDJ4?=
 =?utf-8?B?K2VnbUhjcGIxUU9YL214UmpCbEdRV3FBcWxNMXZOZVdWd0FFMFRIZC85UkJz?=
 =?utf-8?B?WldMUEcvdzN5RHZERkpERUVHbGJ2ZGRxTlBPQnhPVXUvcTdGNVBkTXBrd3Zo?=
 =?utf-8?B?UTNMeVhUbTdhSTIvcCtVUHJlZkxpOUhLMmRyMTdqZXMyS1Z1ZmdLSDdnSFUr?=
 =?utf-8?B?ZmxIRk9rOVhVLzVnYkdpRjdtSVRLcTZpdW4yNklhbGpsY1gwSU5GN3dHTC95?=
 =?utf-8?B?bEg4Q085QkZSYkZDbjdWM2x2OEx5U1dBR0NkWTN5YkZacDdXdHloRisyUm9V?=
 =?utf-8?B?Uzl2ZXhoa0ZTM3dhYkc5OU4vUERWV21KbCtCWjR1eC9QaUUzM1drWERqQXJN?=
 =?utf-8?B?L2R2U2pTU1ZXRUxlM09wUEZ1WjQrUWl0cmFLc1o3NFVKcUtNazk2a3MvTWFH?=
 =?utf-8?B?bDkwVGNIRDFOUCtUWkFoWnZyK1pRTTdPNGV0YVRod3VpZDVxRk1lcCtnR214?=
 =?utf-8?B?MDBxQ0pick1aY1JXeXYwSDRpam1EeXErYkdPbnFSYlpzSUM0TGRxN0lWeGor?=
 =?utf-8?B?bDJ5VEdXSEpDWXhiTjhyeHBCUUYwbjhMdTFFd3ZQQW1CbGZSM0tSN1RrcjZr?=
 =?utf-8?B?R2RQOUUzajlhZkhXa2M3UGhDREFLeUxCV2sxaTRTRlNRMEI4TmNYUVFXWjN1?=
 =?utf-8?B?N2NRMERLWStkNjNYdWlibWhOWE9rMDdtVXFSNTNvejZCRVFOV1JFL2ptUjVu?=
 =?utf-8?B?SlpaMVJ0aWhYZHRmZ3Q5NU9DbVB3TGhrQUtUN3lUeWVVOWVYV2FZdW1yYVQz?=
 =?utf-8?B?M3hBcFo5dEdyOWVoa21YQmxoR3Z2NE9Lb2FzQzZuckM5OENaZEI4djJRTk1L?=
 =?utf-8?B?aGRVVTA4REpLeE5OMmg2WGdzQ1Z3OEVjMzIzankxSU9SQ2ZDREJPR0lUVHNp?=
 =?utf-8?B?TjNlZ0hTcStSRFhLRDhxbmY5ZmpsbnZERStjOEl5SXpKbUErUWRLZC9MbWVu?=
 =?utf-8?B?TFFNMXBpSVJVUDd5NFE4ZlV3RHdLSjllV0kvTnRNNS94UFBrRUYrU0ZmSE5i?=
 =?utf-8?B?VzU3KzBMQVVkMEFXQk5JSzRtSjdOZGFJZGJLM0puRHpBWHgyUVBQclNCWU14?=
 =?utf-8?B?UTV2bmhCMGJnbGR1VzNMYVQ4WVRpV2xKcnYzTTFaeFlGdEJ5dzlQVHJneHVK?=
 =?utf-8?B?amRKTnRBck5CMjNDc29DbzltWldPdllEeGhZdlA2Tm9OcDdUMzMxTlRRU1ly?=
 =?utf-8?B?alVBbGxhdUZ1eWNsMXFHNjg4VWE2c1Nzdzd2Ykx5ekhJRHpPMWJBSDFSMU42?=
 =?utf-8?B?Wm9oeEVEOXovNGFZc1RWaDRXOTNYQXgvUEFJbFd6UDJQL3RVKzhvUldUaEtD?=
 =?utf-8?B?a1k1TlF2LzhWOUNNQjcrdFNJdjZZUXJVYWhTems5U2E4ZERZdGsrczBpZUhB?=
 =?utf-8?B?ZVI5aG83SGswMUxQazAwcGNvL1l6dmVwWWR2eTVYNFlXTUdoZG0yNGpaVm1a?=
 =?utf-8?B?RzFjZEF1a00xajV4aWs4WEd3UU1rdWhOVGtPUUEvUkFsYmQ1dWpLeVVuVnBK?=
 =?utf-8?B?ZW13TW5tSFJIeWxKRnQ0WVAzTUdrQUF0U2poS2hvKzQrcjZKbWRQYTlQQTIx?=
 =?utf-8?B?NDczeEREYTNZdHhqSGZTZHZHZnpLUVNmUURRcnF3elZKelRrYk5Nb1BqeWli?=
 =?utf-8?B?b0FGODZZVXIxOC9UKzZPZXZvOWowTnh1TC9TQjlQQ1pvQUFYWGh6Q2U0cU13?=
 =?utf-8?B?c3grOFVBaStZVitiQ29vVG9kQUtmSjM2bVh4cGpJenBBMHczN0tSazJySXc0?=
 =?utf-8?B?QXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a2d9e9ce-fd66-4151-ea94-08dbfbea144b
X-MS-Exchange-CrossTenant-AuthSource: LO2P265MB4600.GBRP265.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 14:44:58.5801
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0d5fd8bb-e8c2-4e0a-8dd5-2c264f7140fe
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: C4hHqb50eZMuPwBIgybp32iXdFDNXsX0kGTRXOPsyJNuRkeBNL/46y73CwQIP1r4g+hERyx1lRkLQcUgBph62Go3Wr3m5DmmwFrb+kVG3A8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LO0P265MB6535
X-OriginatorOrg: imgtec.com
X-EXCLAIMER-MD-CONFIG: 15a78312-3e47-46eb-9010-2e54d84a9631
X-Proofpoint-GUID: VisxgRGilwb-l1GAxT7Qt03AVJ-pD4pi
X-Proofpoint-ORIG-GUID: VisxgRGilwb-l1GAxT7Qt03AVJ-pD4pi
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 13 Dec 2023 06:47:01 -0800 (PST)

It is possible to double free the vm_ctx->mmu_ctx object in this
function.

    630 err_page_table_destroy:
--> 631         pvr_mmu_context_destroy(vm_ctx->mmu_ctx);

The pvr_vm_context_put() function does:

        kref_put(&vm_ctx->ref_count, pvr_vm_context_release);

Here the pvr_vm_context_release() will call:

        pvr_mmu_context_destroy(vm_ctx->mmu_ctx);

Refactor to an unwind style.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Donald Robson <donald.robson@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_vm.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index 432168aba577..93155eccc50d 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -598,23 +598,12 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
 	if (!vm_ctx)
 		return ERR_PTR(-ENOMEM);
 
-	drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0);
-
 	vm_ctx->pvr_dev = pvr_dev;
-	kref_init(&vm_ctx->ref_count);
-	mutex_init(&vm_ctx->lock);
-
-	drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
-		       is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
-		       0, &pvr_dev->base, &vm_ctx->dummy_gem,
-		       0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
 
 	vm_ctx->mmu_ctx = pvr_mmu_context_create(pvr_dev);
 	err = PTR_ERR_OR_ZERO(vm_ctx->mmu_ctx);
-	if (err) {
-		vm_ctx->mmu_ctx = NULL;
-		goto err_put_ctx;
-	}
+	if (err)
+		goto err_free;
 
 	if (is_userspace_context) {
 		err = pvr_fw_object_create(pvr_dev, sizeof(struct rogue_fwif_fwmemcontext),
@@ -625,13 +614,22 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
 			goto err_page_table_destroy;
 	}
 
+	drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0);
+	drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
+		       is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
+		       0, &pvr_dev->base, &vm_ctx->dummy_gem,
+		       0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
+
+	mutex_init(&vm_ctx->lock);
+	kref_init(&vm_ctx->ref_count);
+
 	return vm_ctx;
 
 err_page_table_destroy:
 	pvr_mmu_context_destroy(vm_ctx->mmu_ctx);
 
-err_put_ctx:
-	pvr_vm_context_put(vm_ctx);
+err_free:
+	kfree(vm_ctx);
 
 	return ERR_PTR(err);
 }
-- 
2.25.1