Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0d292d11 authored by Sumukh Hallymysore Ravindra's avatar Sumukh Hallymysore Ravindra Committed by Gerrit - the friendly Code Review server
Browse files

msm: synx: Prevent accessing valid but released handles 



Release function removes the reference taken during
create, but same release function can be invoked
again to take away the valid reference held by
other threads, which got these references before
release was invoked the first time. This could lead
to successive release call/s taking away reference
held by other threads leading to UAF.

Change adds another member variable to fail release
function even though it is still valid, if the
rel_count member has reached zero. rel_count variable
is set to 1 during synx creation and decremented on
release. All functions after release will fail, as
per design and avoid taking away the reference of
pending functions on same handle.

Change-Id: Ie7ab279bc1e08e82c394b704e181716b726bcf4e
Signed-off-by: default avatarSumukh Hallymysore Ravindra <shallymy@codeaurora.org>
parent e69f8d02

drivers/media/platform/msm/synx/synx.c

+28 −16
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0-only

/*

 * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.

 * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved.

 */

#define pr_fmt(fmt) "synx: " fmt
@@ -681,9 +681,9 @@ EXPORT_SYMBOL(synx_merge); 
int synx_release(struct synx_session session_id, s32 h_synx)

{

	int rc = 0;

	u32 idx;

	struct synx_client *client;

	struct synx_handle_coredata *synx_data;

	struct synx_coredata *synx_obj;



	pr_debug("[sess: %u] Enter release from pid %d\n",

		session_id.client_id, current->pid);
@@ -692,21 +692,31 @@ int synx_release(struct synx_session session_id, s32 h_synx) 
	if (!client)

		return -EINVAL;



	synx_data = synx_util_acquire_handle(client, h_synx);

	synx_obj = synx_util_obtain_object(synx_data);

	/* no need to check for fence here */

	if (!synx_obj) {

		pr_err("%s: [sess: %u] invalid handle access %d\n",

			__func__, client->id, h_synx);

		rc = -EINVAL;

		goto fail;

	}

	synx_data = NULL;

	idx = synx_util_handle_index(h_synx);



	mutex_lock(&client->synx_table_lock[idx]);

	synx_data = &client->synx_table[idx];

	if (!synx_data->synx_obj) {

		pr_err("[sess: %u] invalid object handle %d\n",

			client->id, h_synx);

	} else if (synx_data->handle != h_synx) {

		pr_err("[sess: %u] stale object handle %d\n",

			client->id, h_synx);

	} else if (synx_data->rel_count == 0) {

		pr_err("[sess: %u] released object handle %d\n",

			client->id, h_synx);

	} else if (!kref_read(&synx_data->internal_refcount)) {

		pr_err("[sess: %u] destroyed object handle %d\n",

			client->id, h_synx);

	} else {

		synx_data->rel_count--;

		/* release the reference obtained at synx creation */

	synx_util_release_handle(synx_data);

		kref_put(&synx_data->internal_refcount,

			synx_util_destroy_internal_handle);

	}

	mutex_unlock(&client->synx_table_lock[idx]);



fail:

	synx_util_release_handle(synx_data);

	synx_put_client(client);

	pr_debug("[sess: %u] exit release with status %d\n",

		session_id.client_id, rc);
@@ -953,8 +963,10 @@ int synx_addrefcount(struct synx_session session_id, s32 h_synx, s32 count) 
	idx = synx_util_handle_index(h_synx);

	mutex_lock(&client->synx_table_lock[idx]);

	/* acquire additional references to handle */

	while (count--)

	while (count--) {

		synx_data->rel_count++;

		kref_get(&synx_data->internal_refcount);

	}

	mutex_unlock(&client->synx_table_lock[idx]);



fail:

drivers/media/platform/msm/synx/synx_private.h

+3 −1
Original line number Diff line number Diff line 
/* SPDX-License-Identifier: GPL-2.0-only */

/*

 * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.

 * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved.

 */



#ifndef __SYNX_PRIVATE_H__
@@ -181,6 +181,7 @@ struct synx_device; 
 * @import_refcount   : References for external clients for import

 * @id                : Synx object handle for the client

 * @key               : Key for import authentication

 * @rel_count         : No of allowed release counts

 */

struct synx_handle_coredata {

	struct synx_client *client;
@@ -189,6 +190,7 @@ struct synx_handle_coredata { 
	struct kref import_refcount;

	u32 handle;

	u16 key;

	u32 rel_count;

};



/**

drivers/media/platform/msm/synx/synx_util.c

+4 −0
Original line number Diff line number Diff line
@@ -231,6 +231,7 @@ int synx_util_init_handle(struct synx_client *client, 
	synx_data->handle = h_synx;

	synx_data->synx_obj = synx_obj;

	kref_init(&synx_data->internal_refcount);

	synx_data->rel_count = 1;

	mutex_unlock(&client->synx_table_lock[idx]);



	*new_synx = h_synx;
@@ -570,6 +571,9 @@ struct synx_handle_coredata *synx_util_acquire_handle( 
	} else if (synx_data->handle != h_synx) {

		pr_err("[sess: %u] stale object handle %d\n",

			client->id, h_synx);

	} else if (synx_data->rel_count == 0) {

		pr_err("[sess: %u] released object handle %d\n",

			client->id, h_synx);

	} else if (!kref_read(&synx_data->internal_refcount)) {

		pr_err("[sess: %u] destroyed object handle %d\n",

			client->id, h_synx);