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

Commit 78dbe61a authored by Ashish Jain's avatar Ashish Jain Committed by Gerrit - the friendly Code Review server
Browse files

soc: qcom: fix race condition while freeing private data 



WDSP private data structure is freed in wdsp_glink_release()
but some of the member variables like work_queue pointer is
accessed even after free. Fix this issue by making sure that
glink callback functions are finished execution
before freeing up wdsp private data structure.

Change-Id: Ia4dd9d667109168874dc9188d70741cb9541b0c6
Signed-off-by: default avatarVidyakumar Athota <vathota@codeaurora.org>
Signed-off-by: default avatarAshish Jain <ashishj@codeaurora.org>
parent b9806258

drivers/soc/qcom/wcd-dsp-glink.c

+50 −34
Original line number Diff line number Diff line 
/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved.

/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.

 *

 * This program is free software; you can redistribute it and/or modify

 * it under the terms of the GNU General Public License version 2 and
@@ -89,6 +89,9 @@ struct wdsp_glink_ch { 
	/* Wait for ch connect state before sending any command */

	wait_queue_head_t ch_connect_wait;



	/* Wait for ch local and remote disconnect before channel free */

	wait_queue_head_t ch_free_wait;



	/*

	 * Glink channel configuration. This has to be the last

	 * member of the strucuture as it has variable size
@@ -338,7 +341,7 @@ static void wdsp_glink_notify_state(void *handle, const void *priv, 
	mutex_lock(&ch->mutex);

	ch->channel_state = event;

	if (event == GLINK_CONNECTED) {

		dev_dbg(wpriv->dev, "%s: glink channel: %s connected\n",

		dev_info(wpriv->dev, "%s: glink channel: %s connected\n",

			__func__, ch->ch_cfg.name);



		for (i = 0; i < ch->ch_cfg.no_of_intents; i++) {
@@ -360,31 +363,29 @@ static void wdsp_glink_notify_state(void *handle, const void *priv, 
				ch->ch_cfg.name);



		wake_up(&ch->ch_connect_wait);

		mutex_unlock(&ch->mutex);

	} else if (event == GLINK_LOCAL_DISCONNECTED) {

		/*

		 * Don't use dev_dbg here as dev may not be valid if channel

		 * closed from driver close.

		 */

		pr_debug("%s: channel: %s disconnected locally\n",

		pr_info("%s: channel: %s disconnected locally\n",

			 __func__, ch->ch_cfg.name);

		mutex_unlock(&ch->mutex);



		if (ch->free_mem) {

			kfree(ch);

			ch = NULL;

		}

		ch->free_mem = true;

		wake_up(&ch->ch_free_wait);

		return;

	} else if (event == GLINK_REMOTE_DISCONNECTED) {

		dev_dbg(wpriv->dev, "%s: remote channel: %s disconnected remotely\n",

		pr_info("%s: remote channel: %s disconnected remotely\n",

			 __func__, ch->ch_cfg.name);

		mutex_unlock(&ch->mutex);

		/*

		 * If remote disconnect happens, local side also has

		 * to close the channel as per glink design in a

		 * separate work_queue.

		 */

		if (wpriv && wpriv->work_queue != NULL)

			queue_work(wpriv->work_queue, &ch->lcl_ch_cls_wrk);

	}

	mutex_unlock(&ch->mutex);

}



/*
@@ -399,11 +400,11 @@ static int wdsp_glink_close_ch(struct wdsp_glink_ch *ch) 
	mutex_lock(&wpriv->glink_mutex);

	if (ch->handle) {

		ret = glink_close(ch->handle);

		ch->handle = NULL;

		if (IS_ERR_VALUE(ret)) {

			dev_err(wpriv->dev, "%s: glink_close is failed, ret = %d\n",

				 __func__, ret);

		} else {

			ch->handle = NULL;

			dev_dbg(wpriv->dev, "%s: ch %s is closed\n", __func__,

				ch->ch_cfg.name);

		}
@@ -451,6 +452,7 @@ static int wdsp_glink_open_ch(struct wdsp_glink_ch *ch) 
			ch->handle = NULL;

			ret = -EINVAL;

		}

		ch->free_mem = false;

	} else {

		dev_err(wpriv->dev, "%s: ch %s is already opened\n", __func__,

			ch->ch_cfg.name);
@@ -492,7 +494,7 @@ static int wdsp_glink_open_all_ch(struct wdsp_glink_priv *wpriv) 


err_open:

	for (j = 0; j < i; j++)

		if (wpriv->ch[i])

		if (wpriv->ch[j])

			wdsp_glink_close_ch(wpriv->ch[j]);



done:
@@ -631,6 +633,7 @@ static int wdsp_glink_ch_info_init(struct wdsp_glink_priv *wpriv, 
			goto err_ch_mem;

		}

		ch[i]->channel_state = GLINK_LOCAL_DISCONNECTED;

		ch[i]->free_mem = true;

		memcpy(&ch[i]->ch_cfg, payload, ch_cfg_size);

		payload += ch_cfg_size;
@@ -654,6 +657,7 @@ static int wdsp_glink_ch_info_init(struct wdsp_glink_priv *wpriv, 
		INIT_WORK(&ch[i]->lcl_ch_open_wrk, wdsp_glink_lcl_ch_open_wrk);

		INIT_WORK(&ch[i]->lcl_ch_cls_wrk, wdsp_glink_lcl_ch_cls_wrk);

		init_waitqueue_head(&ch[i]->ch_connect_wait);

		init_waitqueue_head(&ch[i]->ch_free_wait);

	}



	INIT_WORK(&wpriv->ch_open_cls_wrk, wdsp_glink_ch_open_cls_wrk);
@@ -1060,36 +1064,48 @@ static int wdsp_glink_release(struct inode *inode, struct file *file) 
		goto done;

	}



	dev_info(wpriv->dev, "%s: closing wdsp_glink driver\n", __func__);

	if (wpriv->glink_state.handle)

		glink_unregister_link_state_cb(wpriv->glink_state.handle);



	flush_workqueue(wpriv->work_queue);

	destroy_workqueue(wpriv->work_queue);



	/*

	 * Clean up glink channel memory in channel state

	 * callback only if close channels are called from here.

	 * Wait for channel local and remote disconnect state notifications

	 * before freeing channel memory.

	 */

	if (wpriv->ch) {

	for (i = 0; i < wpriv->no_of_channels; i++) {

			if (wpriv->ch[i]) {

				wpriv->ch[i]->free_mem = true;

		if (wpriv->ch && wpriv->ch[i]) {

			/*

				 * Channel handle NULL means channel is already

				 * closed. Free the channel memory here itself.

			 * Only close glink channel from here if REMOTE has

			 * not already disconnected it

			 */

				if (!wpriv->ch[i]->handle) {

					kfree(wpriv->ch[i]);

					wpriv->ch[i] = NULL;

				} else {

			wdsp_glink_close_ch(wpriv->ch[i]);



			ret = wait_event_timeout(wpriv->ch[i]->ch_free_wait,

					(wpriv->ch[i]->free_mem == true),

					msecs_to_jiffies(TIMEOUT_MS));

			if (!ret) {

				pr_err("%s: glink ch %s failed to notify states properly %d\n",

					__func__, wpriv->ch[i]->ch_cfg.name,

					wpriv->ch[i]->channel_state);

				ret = -EINVAL;

				goto done;

			}

		}

	}



	flush_workqueue(wpriv->work_queue);

	destroy_workqueue(wpriv->work_queue);

	wpriv->work_queue = NULL;



	for (i = 0; i < wpriv->no_of_channels; i++) {

		if (wpriv->ch && wpriv->ch[i]) {

			kfree(wpriv->ch[i]);

			wpriv->ch[i] = NULL;

		}

	}

	kfree(wpriv->ch);

	wpriv->ch = NULL;

	}



	mutex_destroy(&wpriv->glink_mutex);

	mutex_destroy(&wpriv->rsp_mutex);