diff mbox series

[v3,6/6] bus: mhi: core: Add range checks for BHI and BHIe

Message ID 1620234501-30461-7-git-send-email-bbhatt@codeaurora.org
State Superseded
Headers show
Series [v3,1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation | expand

Commit Message

Bhaumik Bhatt May 5, 2021, 5:08 p.m. UTC
When obtaining the BHI or BHIe offsets during the power up
preparation phase, range checks are missing. These can help
controller drivers avoid accessing any address outside of the
MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI
registration as it is a required field and range checks will
fail without it.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/bus/mhi/core/init.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Hemant Kumar May 6, 2021, 12:12 a.m. UTC | #1
Hi Bhaumik,

On 5/5/21 10:08 AM, Bhaumik Bhatt wrote:
> When obtaining the BHI or BHIe offsets during the power up

> preparation phase, range checks are missing. These can help

> controller drivers avoid accessing any address outside of the

> MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI

> registration as it is a required field and range checks will

> fail without it.

> 

> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

> ---

>   drivers/bus/mhi/core/init.c | 15 ++++++++++++++-

>   1 file changed, 14 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c

> index 1cc2f22..86ad06e 100644

> --- a/drivers/bus/mhi/core/init.c

> +++ b/drivers/bus/mhi/core/init.c

> @@ -885,7 +885,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,

>   	if (!mhi_cntrl || !mhi_cntrl->cntrl_dev || !mhi_cntrl->regs ||

>   	    !mhi_cntrl->runtime_get || !mhi_cntrl->runtime_put ||

>   	    !mhi_cntrl->status_cb || !mhi_cntrl->read_reg ||

> -	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs || !mhi_cntrl->irq)

> +	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs ||

> +	    !mhi_cntrl->irq || !mhi_cntrl->reg_len)

>   		return -EINVAL;

>   

>   	ret = parse_config(mhi_cntrl, config);

> @@ -1077,6 +1078,12 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)

>   		dev_err(dev, "Error getting BHI offset\n");

>   		goto error_reg_offset;

>   	}

> +

> +	if (bhi_off >= mhi_cntrl->reg_len) {

> +		dev_err(dev, "BHI offset is out of range\n");

Does is make sense to also log bhi_off and/or reg_len values in error if 
it helps in debugging
> +		ret = -EINVAL;

> +		goto error_reg_offset;

> +	}

>   	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;

>   

>   	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {

> @@ -1086,6 +1093,12 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)

>   			dev_err(dev, "Error getting BHIE offset\n");

>   			goto error_reg_offset;

>   		}

> +

> +		if (bhie_off >= mhi_cntrl->reg_len) {

> +			dev_err(dev, "BHIe offset is out of range\n");

Same comment as above
> +			ret = -EINVAL;

> +			goto error_reg_offset;

> +		}

>   		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;

>   	}

>   

> 


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

Patch

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 1cc2f22..86ad06e 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -885,7 +885,8 @@  int mhi_register_controller(struct mhi_controller *mhi_cntrl,
 	if (!mhi_cntrl || !mhi_cntrl->cntrl_dev || !mhi_cntrl->regs ||
 	    !mhi_cntrl->runtime_get || !mhi_cntrl->runtime_put ||
 	    !mhi_cntrl->status_cb || !mhi_cntrl->read_reg ||
-	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs || !mhi_cntrl->irq)
+	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs ||
+	    !mhi_cntrl->irq || !mhi_cntrl->reg_len)
 		return -EINVAL;
 
 	ret = parse_config(mhi_cntrl, config);
@@ -1077,6 +1078,12 @@  int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 		dev_err(dev, "Error getting BHI offset\n");
 		goto error_reg_offset;
 	}
+
+	if (bhi_off >= mhi_cntrl->reg_len) {
+		dev_err(dev, "BHI offset is out of range\n");
+		ret = -EINVAL;
+		goto error_reg_offset;
+	}
 	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
 
 	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
@@ -1086,6 +1093,12 @@  int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 			dev_err(dev, "Error getting BHIE offset\n");
 			goto error_reg_offset;
 		}
+
+		if (bhie_off >= mhi_cntrl->reg_len) {
+			dev_err(dev, "BHIe offset is out of range\n");
+			ret = -EINVAL;
+			goto error_reg_offset;
+		}
 		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
 	}