Re: 答复: [PATCH] bus: mhi: core: Fix null pointer access

From: Bhaumik Bhatt
Date: Wed Oct 28 2020 - 17:58:59 EST


On 2020-10-27 19:03, Carl Yin wrote:
Hi bbhatt:

On Wednesday, October 28, 2020 9:02 AM, bbhatt wrote:

Hi Carl,

Yes this change is needed. Good catch. I ran in to this issue as well when a
dev_err() call was made with a bad MHI configuration.
[carl.yin] yes, I also meet this error with a bad MHI configuration.

Maybe you can explain a little more in the commit text subject?

You could say, "Fix null pointer access when parsing MHI configuration"?

On 2020-10-26 22:33, carl.yin@xxxxxxxxxxx wrote:
> From: carl <carl.yin@xxxxxxxxxxx>
>
> function parse_ev_cfg and parse_ch_cfg access mhi_cntrl->mhi_dev
Functions parse_ev_cfg() and parse_ch_cfg()
> before it is set in function mhi_register_controller,
mhi_register_controller()
> use cntrl_dev to instead mhi_dev.
use cntrl_dev instead of mhi_dev.
>
> Signed-off-by: carl <carl.yin@xxxxxxxxxxx>
With these commit text updates,
[carl.yin] thank for words correction, for my mother language is not English,
there are lots of words wrong in the commit.

No problem.
Reviewed-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
> ---
> drivers/bus/mhi/core/init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 0ffdebde8..c6b43e90b 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -610,7 +610,7 @@ static int parse_ev_cfg(struct mhi_controller
> *mhi_cntrl, {
> struct mhi_event *mhi_event;
> const struct mhi_event_config *event_cfg;
> - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + struct device *dev = mhi_cntrl->cntrl_dev;
> int i, num;
>
> num = config->num_events;
> @@ -692,7 +692,7 @@ static int parse_ch_cfg(struct mhi_controller
> *mhi_cntrl,
> const struct mhi_controller_config *config) {
> const struct mhi_channel_config *ch_cfg;
> - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + struct device *dev = mhi_cntrl->cntrl_dev;
> int i;
> u32 chan;

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

Can you also add a "Fixes:" tag to the patch?

Refer Documentation/process/submitting-patches.rst:

If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary. Do not split the tag across multiple
lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
parsing scripts. For example::

Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")

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