Re: [PATCH v2 2/8] bus: mhi: core: Add range check for channel id received in event ring

From: Hemant Kumar
Date: Wed Apr 29 2020 - 13:29:38 EST


Hi Jeff

On 4/28/20 7:44 AM, Jeffrey Hugo wrote:
On 4/27/2020 8:59 PM, Bhaumik Bhatt wrote:
From: Hemant Kumar <hemantk@xxxxxxxxxxxxxx>

MHI data completion handler function reads channel id from event
ring element. Value is under the control of MHI devices and can be
any value between 0 and 255. In order to prevent out of bound access
add a bound check against the max channel supported by controller
and skip processing of that event ring element.

Signed-off-by: Hemant Kumar <hemantk@xxxxxxxxxxxxxx>
Signed-off-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
---
 drivers/bus/mhi/core/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 23154f1..1ccd4cc 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -827,6 +827,9 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl,
ÂÂÂÂÂÂÂÂÂ enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
ÂÂÂÂÂÂÂÂÂ chan = MHI_TRE_GET_EV_CHID(local_rp);
+ÂÂÂÂÂÂÂ if (WARN_ON(chan >= mhi_cntrl->max_chan))
+ÂÂÂÂÂÂÂÂÂÂÂ goto next_event;
+
ÂÂÂÂÂÂÂÂÂ mhi_chan = &mhi_cntrl->mhi_chan[chan];
ÂÂÂÂÂÂÂÂÂ if (likely(type == MHI_PKT_TYPE_TX_EVENT)) {
@@ -837,6 +840,7 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl,
ÂÂÂÂÂÂÂÂÂÂÂÂÂ event_quota--;
ÂÂÂÂÂÂÂÂÂ }
+next_event:
ÂÂÂÂÂÂÂÂÂ mhi_recycle_ev_ring_element(mhi_cntrl, ev_ring);
ÂÂÂÂÂÂÂÂÂ local_rp = ev_ring->rp;
ÂÂÂÂÂÂÂÂÂ dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);


It looks like the same issue exists in mhi_process_ctrl_ev_ring(), and thus some form of this solution needs to be applied there as well. Would you please fix that too?

As discussed with you off line, spec allows to have just event ring to be used for both data and control. Updating this in V3.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project