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

Commit 44f6df92 authored by Sricharan R's avatar Sricharan R Committed by Bjorn Andersson
Browse files

rpmsg: glink: Fix idr_lock from mutex to spinlock 



The channel members lcids, rcids synchronised using the idr_lock is
accessed in both atomic/non-atomic contexts. The readers are not
currently synchronised.  That no correct, so add the readers as well
under the lock and use a spinlock.

Acked-by: default avatarArun Kumar Neelakantam <aneela@codeaurora.org>
Signed-off-by: default avatarSricharan R <sricharan@codeaurora.org>
Signed-off-by: default avatarBjorn Andersson <bjorn.andersson@linaro.org>
parent d31ad615

drivers/rpmsg/qcom_glink_native.c

+42 −20
Original line number Original line Diff line number Diff line
@@ -92,7 +92,7 @@ struct qcom_glink { 




	struct mutex tx_lock;

	struct mutex tx_lock;





	struct mutex idr_lock;

	spinlock_t idr_lock;

	struct idr lcids;

	struct idr lcids;

	struct idr rcids;

	struct idr rcids;

	unsigned long features;

	unsigned long features;
@@ -309,14 +309,15 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink, 
	int name_len = strlen(channel->name) + 1;

	int name_len = strlen(channel->name) + 1;

	int req_len = ALIGN(sizeof(req.msg) + name_len, 8);

	int req_len = ALIGN(sizeof(req.msg) + name_len, 8);

	int ret;

	int ret;

	unsigned long flags;





	kref_get(&channel->refcount);

	kref_get(&channel->refcount);





	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	ret = idr_alloc_cyclic(&glink->lcids, channel,

	ret = idr_alloc_cyclic(&glink->lcids, channel,

			       RPM_GLINK_CID_MIN, RPM_GLINK_CID_MAX,

			       RPM_GLINK_CID_MIN, RPM_GLINK_CID_MAX,

			       GFP_KERNEL);

			       GFP_ATOMIC);

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);

	if (ret < 0)

	if (ret < 0)

		return ret;

		return ret;
@@ -334,10 +335,10 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink, 
	return 0;

	return 0;





remove_idr:

remove_idr:

	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_remove(&glink->lcids, channel->lcid);

	idr_remove(&glink->lcids, channel->lcid);

	channel->lcid = 0;

	channel->lcid = 0;

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	return ret;

	return ret;

}

}
@@ -463,6 +464,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) 
	unsigned int chunk_size;

	unsigned int chunk_size;

	unsigned int left_size;

	unsigned int left_size;

	unsigned int rcid;

	unsigned int rcid;

	unsigned long flags;





	if (avail < sizeof(hdr)) {

	if (avail < sizeof(hdr)) {

		dev_dbg(glink->dev, "Not enough data in fifo\n");

		dev_dbg(glink->dev, "Not enough data in fifo\n");
@@ -482,7 +484,9 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) 
		return -EINVAL;

		return -EINVAL;





	rcid = le16_to_cpu(hdr.msg.param1);

	rcid = le16_to_cpu(hdr.msg.param1);

	spin_lock_irqsave(&glink->idr_lock, flags);

	channel = idr_find(&glink->rcids, rcid);

	channel = idr_find(&glink->rcids, rcid);

	spin_unlock_irqrestore(&glink->idr_lock, flags);

	if (!channel) {

	if (!channel) {

		dev_dbg(glink->dev, "Data on non-existing channel\n");

		dev_dbg(glink->dev, "Data on non-existing channel\n");
@@ -543,11 +547,13 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) 
{

{

	struct glink_channel *channel;

	struct glink_channel *channel;





	spin_lock(&glink->idr_lock);

	channel = idr_find(&glink->lcids, lcid);

	channel = idr_find(&glink->lcids, lcid);

	if (!channel) {

	if (!channel) {

		dev_err(glink->dev, "Invalid open ack packet\n");

		dev_err(glink->dev, "Invalid open ack packet\n");

		return -EINVAL;

		return -EINVAL;

	}

	}

	spin_unlock(&glink->idr_lock);





	complete(&channel->open_ack);

	complete(&channel->open_ack);
@@ -620,6 +626,7 @@ static struct glink_channel *qcom_glink_create_local(struct qcom_glink *glink, 
{

{

	struct glink_channel *channel;

	struct glink_channel *channel;

	int ret;

	int ret;

	unsigned long flags;





	channel = qcom_glink_alloc_channel(glink, name);

	channel = qcom_glink_alloc_channel(glink, name);

	if (IS_ERR(channel))

	if (IS_ERR(channel))
@@ -643,9 +650,9 @@ static struct glink_channel *qcom_glink_create_local(struct qcom_glink *glink, 




err_timeout:

err_timeout:

	/* qcom_glink_send_open_req() did register the channel in lcids*/

	/* qcom_glink_send_open_req() did register the channel in lcids*/

	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_remove(&glink->lcids, channel->lcid);

	idr_remove(&glink->lcids, channel->lcid);

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);





release_channel:

release_channel:

	/* Release qcom_glink_send_open_req() reference */

	/* Release qcom_glink_send_open_req() reference */
@@ -702,11 +709,14 @@ static struct rpmsg_endpoint *qcom_glink_create_ept(struct rpmsg_device *rpdev, 
	const char *name = chinfo.name;

	const char *name = chinfo.name;

	int cid;

	int cid;

	int ret;

	int ret;

	unsigned long flags;





	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_for_each_entry(&glink->rcids, channel, cid) {

	idr_for_each_entry(&glink->rcids, channel, cid) {

		if (!strcmp(channel->name, name))

		if (!strcmp(channel->name, name))

			break;

			break;

	}

	}

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	if (!channel) {

	if (!channel) {

		channel = qcom_glink_create_local(glink, name);

		channel = qcom_glink_create_local(glink, name);
@@ -828,11 +838,14 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid, 
	struct device_node *node;

	struct device_node *node;

	int lcid;

	int lcid;

	int ret;

	int ret;

	unsigned long flags;





	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_for_each_entry(&glink->lcids, channel, lcid) {

	idr_for_each_entry(&glink->lcids, channel, lcid) {

		if (!strcmp(channel->name, name))

		if (!strcmp(channel->name, name))

			break;

			break;

	}

	}

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	if (!channel) {

	if (!channel) {

		channel = qcom_glink_alloc_channel(glink, name);

		channel = qcom_glink_alloc_channel(glink, name);
@@ -843,15 +856,15 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid, 
		create_device = true;

		create_device = true;

	}

	}





	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	ret = idr_alloc(&glink->rcids, channel, rcid, rcid + 1, GFP_KERNEL);

	ret = idr_alloc(&glink->rcids, channel, rcid, rcid + 1, GFP_ATOMIC);

	if (ret < 0) {

	if (ret < 0) {

		dev_err(glink->dev, "Unable to insert channel into rcid list\n");

		dev_err(glink->dev, "Unable to insert channel into rcid list\n");

		mutex_unlock(&glink->idr_lock);

		spin_unlock_irqrestore(&glink->idr_lock, flags);

		goto free_channel;

		goto free_channel;

	}

	}

	channel->rcid = ret;

	channel->rcid = ret;

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	complete(&channel->open_req);

	complete(&channel->open_req);
@@ -885,10 +898,10 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid, 
free_rpdev:

free_rpdev:

	kfree(rpdev);

	kfree(rpdev);

rcid_remove:

rcid_remove:

	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_remove(&glink->rcids, channel->rcid);

	idr_remove(&glink->rcids, channel->rcid);

	channel->rcid = 0;

	channel->rcid = 0;

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);

free_channel:

free_channel:

	/* Release the reference, iff we took it */

	/* Release the reference, iff we took it */

	if (create_device)

	if (create_device)
@@ -901,8 +914,11 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid) 
{

{

	struct rpmsg_channel_info chinfo;

	struct rpmsg_channel_info chinfo;

	struct glink_channel *channel;

	struct glink_channel *channel;

	unsigned long flags;





	spin_lock_irqsave(&glink->idr_lock, flags);

	channel = idr_find(&glink->rcids, rcid);

	channel = idr_find(&glink->rcids, rcid);

	spin_unlock_irqrestore(&glink->idr_lock, flags);

	if (WARN(!channel, "close request on unknown channel\n"))

	if (WARN(!channel, "close request on unknown channel\n"))

		return;

		return;
@@ -916,10 +932,10 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid) 




	qcom_glink_send_close_ack(glink, channel->rcid);

	qcom_glink_send_close_ack(glink, channel->rcid);





	mutex_lock(&glink->idr_lock);

	spin_lock_irqsave(&glink->idr_lock, flags);

	idr_remove(&glink->rcids, channel->rcid);

	idr_remove(&glink->rcids, channel->rcid);

	channel->rcid = 0;

	channel->rcid = 0;

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	kref_put(&channel->refcount, qcom_glink_channel_release);

	kref_put(&channel->refcount, qcom_glink_channel_release);

}

}
@@ -927,15 +943,18 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid) 
static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)

static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)

{

{

	struct glink_channel *channel;

	struct glink_channel *channel;

	unsigned long flags;





	spin_lock_irqsave(&glink->idr_lock, flags);

	channel = idr_find(&glink->lcids, lcid);

	channel = idr_find(&glink->lcids, lcid);

	if (WARN(!channel, "close ack on unknown channel\n"))

	if (WARN(!channel, "close ack on unknown channel\n")) {

		spin_unlock_irqrestore(&glink->idr_lock, flags);

		return;

		return;

	}





	mutex_lock(&glink->idr_lock);

	idr_remove(&glink->lcids, channel->lcid);

	idr_remove(&glink->lcids, channel->lcid);

	channel->lcid = 0;

	channel->lcid = 0;

	mutex_unlock(&glink->idr_lock);

	spin_unlock_irqrestore(&glink->idr_lock, flags);





	kref_put(&channel->refcount, qcom_glink_channel_release);

	kref_put(&channel->refcount, qcom_glink_channel_release);

}

}
@@ -1016,7 +1035,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev, 
	INIT_LIST_HEAD(&glink->rx_queue);

	INIT_LIST_HEAD(&glink->rx_queue);

	INIT_WORK(&glink->rx_work, qcom_glink_work);

	INIT_WORK(&glink->rx_work, qcom_glink_work);





	mutex_init(&glink->idr_lock);

	spin_lock_init(&glink->idr_lock);

	idr_init(&glink->lcids);

	idr_init(&glink->lcids);

	idr_init(&glink->rcids);

	idr_init(&glink->rcids);
@@ -1059,6 +1078,7 @@ void qcom_glink_native_remove(struct qcom_glink *glink) 
	struct glink_channel *channel;

	struct glink_channel *channel;

	int cid;

	int cid;

	int ret;

	int ret;

	unsigned long flags;





	disable_irq(glink->irq);

	disable_irq(glink->irq);

	cancel_work_sync(&glink->rx_work);

	cancel_work_sync(&glink->rx_work);
@@ -1067,12 +1087,14 @@ void qcom_glink_native_remove(struct qcom_glink *glink) 
	if (ret)

	if (ret)

		dev_warn(glink->dev, "Can't remove GLINK devices: %d\n", ret);

		dev_warn(glink->dev, "Can't remove GLINK devices: %d\n", ret);





	spin_lock_irqsave(&glink->idr_lock, flags);

	/* Release any defunct local channels, waiting for close-ack */

	/* Release any defunct local channels, waiting for close-ack */

	idr_for_each_entry(&glink->lcids, channel, cid)

	idr_for_each_entry(&glink->lcids, channel, cid)

		kref_put(&channel->refcount, qcom_glink_channel_release);

		kref_put(&channel->refcount, qcom_glink_channel_release);





	idr_destroy(&glink->lcids);

	idr_destroy(&glink->lcids);

	idr_destroy(&glink->rcids);

	idr_destroy(&glink->rcids);

	spin_unlock_irqrestore(&glink->idr_lock, flags);

	mbox_free_channel(glink->mbox_chan);

	mbox_free_channel(glink->mbox_chan);

}

}