From 4a348646df68e2c29904545e262c36c26684edf8 Mon Sep 17 00:00:00 2001 From: Pranav Mahesh Phansalkar Date: Thu, 25 Apr 2024 12:22:22 +0530 Subject: [PATCH] rpmsg: glink: Get reference of channel objects in rx path Get channel references in data receive path as channel might get freed while processing commands received from remote processor. This ensures channel context is not freed before its usage is complete. Change-Id: I7d9a98e34c21ae0d277456853a755dab8d105d5f Signed-off-by: Pranav Mahesh Phansalkar --- drivers/rpmsg/qcom_glink_native.c | 85 +++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index b826865862f2..a5caf62682c8 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -2,6 +2,7 @@ /* * Copyright (c) 2016-2017, Linaro Ltd * Copyright (c) 2018-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -354,6 +355,38 @@ static void qcom_glink_channel_release(struct kref *ref) kfree(channel); } +static struct glink_channel *qcom_glink_channel_ref_get( + struct qcom_glink *glink, + bool remote_channel, int cid) +{ + struct glink_channel *channel = NULL; + unsigned long flags; + + if (!glink) + return NULL; + + spin_lock_irqsave(&glink->idr_lock, flags); + if (remote_channel) + channel = idr_find(&glink->rcids, cid); + else + channel = idr_find(&glink->lcids, cid); + + if (channel) + kref_get(&channel->refcount); + + spin_unlock_irqrestore(&glink->idr_lock, flags); + return channel; +} + +static void qcom_glink_channel_ref_put(struct glink_channel *channel) +{ + + if (!channel) + return; + + kref_put(&channel->refcount, qcom_glink_channel_release); +} + static size_t qcom_glink_rx_avail(struct qcom_glink *glink) { return glink->rx_pipe->avail(glink->rx_pipe); @@ -505,11 +538,8 @@ static void qcom_glink_handle_intent_req_ack(struct qcom_glink *glink, unsigned int cid, bool granted) { struct glink_channel *channel; - unsigned long flags; - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, cid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, cid); if (!channel) { dev_err(glink->dev, "unable to find channel\n"); return; @@ -519,6 +549,7 @@ static void qcom_glink_handle_intent_req_ack(struct qcom_glink *glink, atomic_inc(&channel->intent_req_acked); wake_up(&channel->intent_req_ack); CH_INFO(channel, "\n"); + qcom_glink_channel_ref_put(channel); } /** @@ -867,9 +898,7 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink, struct glink_channel *channel; unsigned long flags; - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, cid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, cid); if (!channel) { dev_err(glink->dev, "invalid channel id received\n"); return; @@ -881,6 +910,7 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink, if (!intent) { spin_unlock_irqrestore(&channel->intent_lock, flags); dev_err(glink->dev, "invalid intent id received\n"); + qcom_glink_channel_ref_put(channel); return; } @@ -892,6 +922,7 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink, kfree(intent); } spin_unlock_irqrestore(&channel->intent_lock, flags); + qcom_glink_channel_ref_put(channel); } /** @@ -914,9 +945,7 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink, unsigned long flags; int iid; - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, cid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, cid); if (!channel) { pr_err("%s channel not found for cid %d\n", __func__, cid); @@ -933,6 +962,7 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink, spin_unlock_irqrestore(&channel->intent_lock, flags); if (intent) { qcom_glink_send_intent_req_ack(glink, channel, !!intent); + qcom_glink_channel_ref_put(channel); return; } @@ -942,6 +972,7 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink, qcom_glink_advertise_intent(glink, channel, intent); qcom_glink_send_intent_req_ack(glink, channel, !!intent); + qcom_glink_channel_ref_put(channel); } static int qcom_glink_rx_defer(struct qcom_glink *glink, size_t extra) @@ -976,7 +1007,7 @@ static int qcom_glink_rx_defer(struct qcom_glink *glink, size_t extra) static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) { struct glink_core_rx_intent *intent; - struct glink_channel *channel; + struct glink_channel *channel = NULL; struct { struct glink_msg msg; __le32 chunk_size; @@ -1004,9 +1035,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) } rcid = le16_to_cpu(hdr.msg.param1); - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, rcid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, rcid); if (!channel) { dev_dbg(glink->dev, "Data on non-existing channel\n"); @@ -1019,13 +1048,16 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) /* Might have an ongoing, fragmented, message to append */ if (!channel->buf) { intent = kzalloc(sizeof(*intent), GFP_ATOMIC); - if (!intent) + if (!intent) { + qcom_glink_channel_ref_put(channel); return -ENOMEM; + } intent->data = kmalloc(chunk_size + left_size, GFP_ATOMIC); if (!intent->data) { kfree(intent); + qcom_glink_channel_ref_put(channel); return -ENOMEM; } @@ -1092,7 +1124,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail) advance_rx: qcom_glink_rx_advance(glink, ALIGN(sizeof(hdr) + chunk_size, 8)); - + qcom_glink_channel_ref_put(channel); return ret; } @@ -1123,9 +1155,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, return; } - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, cid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, cid); if (!channel) { dev_err(glink->dev, "intents for non-existing channel\n"); qcom_glink_rx_advance(glink, ALIGN(msglen, 8)); @@ -1133,8 +1163,10 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, } msg = kmalloc(msglen, GFP_ATOMIC); - if (!msg) + if (!msg) { + qcom_glink_channel_ref_put(channel); return; + } qcom_glink_rx_peak(glink, msg, 0, msglen); @@ -1163,15 +1195,14 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, kfree(msg); qcom_glink_rx_advance(glink, ALIGN(msglen, 8)); + qcom_glink_channel_ref_put(channel); } static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) { struct glink_channel *channel; - spin_lock(&glink->idr_lock); - channel = idr_find(&glink->lcids, lcid); - spin_unlock(&glink->idr_lock); + channel = qcom_glink_channel_ref_get(glink, false, lcid); if (!channel) { dev_err(glink->dev, "Invalid open ack packet\n"); return -EINVAL; @@ -1179,7 +1210,7 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) CH_INFO(channel, "\n"); complete_all(&channel->open_ack); - + qcom_glink_channel_ref_put(channel); return 0; } @@ -1220,12 +1251,9 @@ static int qcom_glink_handle_signals(struct qcom_glink *glink, unsigned int rcid, unsigned int signals) { struct glink_channel *channel; - unsigned long flags; u32 old; - spin_lock_irqsave(&glink->idr_lock, flags); - channel = idr_find(&glink->rcids, rcid); - spin_unlock_irqrestore(&glink->idr_lock, flags); + channel = qcom_glink_channel_ref_get(glink, true, rcid); if (!channel) { dev_err(glink->dev, "signal for non-existing channel\n"); return -EINVAL; @@ -1252,6 +1280,7 @@ static int qcom_glink_handle_signals(struct qcom_glink *glink, old, channel->rsigs); } + qcom_glink_channel_ref_put(channel); return 0; }