[PATCH] iommu/qcom:fix local_base status check

[PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

Release resources when exiting on error.

Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 drivers/iommu/qcom_iommu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 4328da0b0..d4ec38b1e 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -815,6 +815,8 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res)
 		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(qcom_iommu->local_base))
+			return PTR_ERR(qcom_iommu->local_base);
 
 	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
 	if (IS_ERR(qcom_iommu->iface_clk)) {
-- 
2.20.1.windows.1

Re: [PATCH] iommu/qcom:fix local_base status check

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9202 bytes --]

Hi Tang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iommu/next]
[also build test WARNING on v5.6 next-20200401]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tang-Bin/iommu-qcom-fix-local_base-status-check/20200402-030758
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/iommu/qcom_iommu.c: In function 'qcom_iommu_device_probe':
>> drivers/iommu/qcom_iommu.c:827:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     827 |  if (res)
         |  ^~
   drivers/iommu/qcom_iommu.c:829:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
     829 |   if (IS_ERR(qcom_iommu->local_base))
         |   ^~

vim +/if +827 drivers/iommu/qcom_iommu.c

d051f28c880712 Stanimir Varbanov   2017-08-09  804  
0ae349a0f33fb0 Rob Clark           2017-08-09  805  static int qcom_iommu_device_probe(struct platform_device *pdev)
0ae349a0f33fb0 Rob Clark           2017-08-09  806  {
0ae349a0f33fb0 Rob Clark           2017-08-09  807  	struct device_node *child;
0ae349a0f33fb0 Rob Clark           2017-08-09  808  	struct qcom_iommu_dev *qcom_iommu;
0ae349a0f33fb0 Rob Clark           2017-08-09  809  	struct device *dev = &pdev->dev;
0ae349a0f33fb0 Rob Clark           2017-08-09  810  	struct resource *res;
8758553791dfb3 Gustavo A. R. Silva 2019-08-29  811  	int ret, max_asid = 0;
0ae349a0f33fb0 Rob Clark           2017-08-09  812  
0ae349a0f33fb0 Rob Clark           2017-08-09  813  	/* find the max asid (which is 1:1 to ctx bank idx), so we know how
0ae349a0f33fb0 Rob Clark           2017-08-09  814  	 * many child ctx devices we have:
0ae349a0f33fb0 Rob Clark           2017-08-09  815  	 */
0ae349a0f33fb0 Rob Clark           2017-08-09  816  	for_each_child_of_node(dev->of_node, child)
0ae349a0f33fb0 Rob Clark           2017-08-09  817  		max_asid = max(max_asid, get_asid(child));
0ae349a0f33fb0 Rob Clark           2017-08-09  818  
8758553791dfb3 Gustavo A. R. Silva 2019-08-29  819  	qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, max_asid),
8758553791dfb3 Gustavo A. R. Silva 2019-08-29  820  				  GFP_KERNEL);
0ae349a0f33fb0 Rob Clark           2017-08-09  821  	if (!qcom_iommu)
0ae349a0f33fb0 Rob Clark           2017-08-09  822  		return -ENOMEM;
0ae349a0f33fb0 Rob Clark           2017-08-09  823  	qcom_iommu->num_ctxs = max_asid;
0ae349a0f33fb0 Rob Clark           2017-08-09  824  	qcom_iommu->dev = dev;
0ae349a0f33fb0 Rob Clark           2017-08-09  825  
0ae349a0f33fb0 Rob Clark           2017-08-09  826  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
0ae349a0f33fb0 Rob Clark           2017-08-09 @827  	if (res)
0ae349a0f33fb0 Rob Clark           2017-08-09  828  		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
09194ae55b9714 Tang Bin            2020-04-01  829  		if (IS_ERR(qcom_iommu->local_base))
09194ae55b9714 Tang Bin            2020-04-01  830  			return PTR_ERR(qcom_iommu->local_base);
0ae349a0f33fb0 Rob Clark           2017-08-09  831  
0ae349a0f33fb0 Rob Clark           2017-08-09  832  	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
0ae349a0f33fb0 Rob Clark           2017-08-09  833  	if (IS_ERR(qcom_iommu->iface_clk)) {
0ae349a0f33fb0 Rob Clark           2017-08-09  834  		dev_err(dev, "failed to get iface clock\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  835  		return PTR_ERR(qcom_iommu->iface_clk);
0ae349a0f33fb0 Rob Clark           2017-08-09  836  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  837  
0ae349a0f33fb0 Rob Clark           2017-08-09  838  	qcom_iommu->bus_clk = devm_clk_get(dev, "bus");
0ae349a0f33fb0 Rob Clark           2017-08-09  839  	if (IS_ERR(qcom_iommu->bus_clk)) {
0ae349a0f33fb0 Rob Clark           2017-08-09  840  		dev_err(dev, "failed to get bus clock\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  841  		return PTR_ERR(qcom_iommu->bus_clk);
0ae349a0f33fb0 Rob Clark           2017-08-09  842  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  843  
0ae349a0f33fb0 Rob Clark           2017-08-09  844  	if (of_property_read_u32(dev->of_node, "qcom,iommu-secure-id",
0ae349a0f33fb0 Rob Clark           2017-08-09  845  				 &qcom_iommu->sec_id)) {
0ae349a0f33fb0 Rob Clark           2017-08-09  846  		dev_err(dev, "missing qcom,iommu-secure-id property\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  847  		return -ENODEV;
0ae349a0f33fb0 Rob Clark           2017-08-09  848  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  849  
d051f28c880712 Stanimir Varbanov   2017-08-09  850  	if (qcom_iommu_has_secure_context(qcom_iommu)) {
d051f28c880712 Stanimir Varbanov   2017-08-09  851  		ret = qcom_iommu_sec_ptbl_init(dev);
d051f28c880712 Stanimir Varbanov   2017-08-09  852  		if (ret) {
d051f28c880712 Stanimir Varbanov   2017-08-09  853  			dev_err(dev, "cannot init secure pg table(%d)\n", ret);
d051f28c880712 Stanimir Varbanov   2017-08-09  854  			return ret;
d051f28c880712 Stanimir Varbanov   2017-08-09  855  		}
d051f28c880712 Stanimir Varbanov   2017-08-09  856  	}
d051f28c880712 Stanimir Varbanov   2017-08-09  857  
0ae349a0f33fb0 Rob Clark           2017-08-09  858  	platform_set_drvdata(pdev, qcom_iommu);
0ae349a0f33fb0 Rob Clark           2017-08-09  859  
0ae349a0f33fb0 Rob Clark           2017-08-09  860  	pm_runtime_enable(dev);
0ae349a0f33fb0 Rob Clark           2017-08-09  861  
0ae349a0f33fb0 Rob Clark           2017-08-09  862  	/* register context bank devices, which are child nodes: */
0ae349a0f33fb0 Rob Clark           2017-08-09  863  	ret = devm_of_platform_populate(dev);
0ae349a0f33fb0 Rob Clark           2017-08-09  864  	if (ret) {
0ae349a0f33fb0 Rob Clark           2017-08-09  865  		dev_err(dev, "Failed to populate iommu contexts\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  866  		return ret;
0ae349a0f33fb0 Rob Clark           2017-08-09  867  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  868  
0ae349a0f33fb0 Rob Clark           2017-08-09  869  	ret = iommu_device_sysfs_add(&qcom_iommu->iommu, dev, NULL,
0ae349a0f33fb0 Rob Clark           2017-08-09  870  				     dev_name(dev));
0ae349a0f33fb0 Rob Clark           2017-08-09  871  	if (ret) {
0ae349a0f33fb0 Rob Clark           2017-08-09  872  		dev_err(dev, "Failed to register iommu in sysfs\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  873  		return ret;
0ae349a0f33fb0 Rob Clark           2017-08-09  874  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  875  
0ae349a0f33fb0 Rob Clark           2017-08-09  876  	iommu_device_set_ops(&qcom_iommu->iommu, &qcom_iommu_ops);
0ae349a0f33fb0 Rob Clark           2017-08-09  877  	iommu_device_set_fwnode(&qcom_iommu->iommu, dev->fwnode);
0ae349a0f33fb0 Rob Clark           2017-08-09  878  
0ae349a0f33fb0 Rob Clark           2017-08-09  879  	ret = iommu_device_register(&qcom_iommu->iommu);
0ae349a0f33fb0 Rob Clark           2017-08-09  880  	if (ret) {
0ae349a0f33fb0 Rob Clark           2017-08-09  881  		dev_err(dev, "Failed to register iommu\n");
0ae349a0f33fb0 Rob Clark           2017-08-09  882  		return ret;
0ae349a0f33fb0 Rob Clark           2017-08-09  883  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  884  
0ae349a0f33fb0 Rob Clark           2017-08-09  885  	bus_set_iommu(&platform_bus_type, &qcom_iommu_ops);
0ae349a0f33fb0 Rob Clark           2017-08-09  886  
0ae349a0f33fb0 Rob Clark           2017-08-09  887  	if (qcom_iommu->local_base) {
0ae349a0f33fb0 Rob Clark           2017-08-09  888  		pm_runtime_get_sync(dev);
0ae349a0f33fb0 Rob Clark           2017-08-09  889  		writel_relaxed(0xffffffff, qcom_iommu->local_base + SMMU_INTR_SEL_NS);
0ae349a0f33fb0 Rob Clark           2017-08-09  890  		pm_runtime_put_sync(dev);
0ae349a0f33fb0 Rob Clark           2017-08-09  891  	}
0ae349a0f33fb0 Rob Clark           2017-08-09  892  
0ae349a0f33fb0 Rob Clark           2017-08-09  893  	return 0;
0ae349a0f33fb0 Rob Clark           2017-08-09  894  }
0ae349a0f33fb0 Rob Clark           2017-08-09  895  

:::::: The code at line 827 was first introduced by commit
:::::: 0ae349a0f33fb040a2bc228fdc6d60111455feab iommu/qcom: Add qcom_iommu

:::::: TO: Rob Clark <robdclark@gmail.com>
:::::: CC: Joerg Roedel <jroedel@suse.de>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 47132 bytes --]

Re: [PATCH] iommu/qcom:fix local_base status check

[Bjorn Andersson]

On Wed 01 Apr 08:20 PDT 2020, Tang Bin wrote:

> Release resources when exiting on error.
> 
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>  drivers/iommu/qcom_iommu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0..d4ec38b1e 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -815,6 +815,8 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (res)
>  		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(qcom_iommu->local_base))

Good catch! But while it works, this is not under the if (res). So
please add some {} around this chunk.

Regards,
Bjorn

> +			return PTR_ERR(qcom_iommu->local_base);
>  
>  	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>  	if (IS_ERR(qcom_iommu->iface_clk)) {
> -- 
> 2.20.1.windows.1
> 
> 
> 

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

On 2020/4/18 19:54, Joerg Roedel wrote:
> On Thu, Apr 16, 2020 at 02:42:23PM +0800, Tang Bin wrote:
>>          The function qcom_iommu_device_probe() does not perform sufficient
>> error checking after executing devm_ioremap_resource(), which can result in
>> crashes if a critical error path is encountered.
>>
>> Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")
> Yes, that sounds better. Please use it for the commit message and also
> add the Fixes line and resubmit the fix to me.
> Please make the fixes line:
>
> 	Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")
>
> So that the commit-id is 12 characters long and a space between it and
> the subject.

Got it, thanks for your instruction.

Tang Bin

>
>

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

On 2020/4/18 19:54, Joerg Roedel wrote:
> On Thu, Apr 16, 2020 at 02:42:23PM +0800, Tang Bin wrote:
>>          The function qcom_iommu_device_probe() does not perform sufficient
>> error checking after executing devm_ioremap_resource(), which can result in
>> crashes if a critical error path is encountered.
>>
>> Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")
> Yes, that sounds better. Please use it for the commit message and also
> add the Fixes line and resubmit the fix to me.
> Please make the fixes line:
>
> 	Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")
>
> So that the commit-id is 12 characters long and a space between it and
> the subject.

Got it, thanks for your instruction.

Tang Bin

>
>


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/qcom:fix local_base status check

[Joerg Roedel]

On Thu, Apr 16, 2020 at 02:42:23PM +0800, Tang Bin wrote:
>         The function qcom_iommu_device_probe() does not perform sufficient
> error checking after executing devm_ioremap_resource(), which can result in
> crashes if a critical error path is encountered.
> 
> Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")

Yes, that sounds better. Please use it for the commit message and also
add the Fixes line and resubmit the fix to me.
Please make the fixes line:

	Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")

So that the commit-id is 12 characters long and a space between it and
the subject.

Thanks,

	Joerg

Re: [PATCH] iommu/qcom:fix local_base status check

[Joerg Roedel]

On Thu, Apr 16, 2020 at 02:42:23PM +0800, Tang Bin wrote:
>         The function qcom_iommu_device_probe() does not perform sufficient
> error checking after executing devm_ioremap_resource(), which can result in
> crashes if a critical error path is encountered.
> 
> Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")

Yes, that sounds better. Please use it for the commit message and also
add the Fixes line and resubmit the fix to me.
Please make the fixes line:

	Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")

So that the commit-id is 12 characters long and a space between it and
the subject.

Thanks,

	Joerg

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

On 2020/4/16 18:05, Robin Murphy wrote:
> On 2020-04-02 7:33 am, Tang Bin wrote:
>> Release resources when exiting on error.
>>
>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>> ---
>>   drivers/iommu/qcom_iommu.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
>> index 4328da0b0..c08aa9651 100644
>> --- a/drivers/iommu/qcom_iommu.c
>> +++ b/drivers/iommu/qcom_iommu.c
>> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct 
>> platform_device *pdev)
>>       qcom_iommu->dev = dev;
>>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    if (res)
>> +    if (res) {
>>           qcom_iommu->local_base = devm_ioremap_resource(dev, res);
>> +        if (IS_ERR(qcom_iommu->local_base))
>> +            return PTR_ERR(qcom_iommu->local_base);
>> +    }
>
> ...or just use devm_platform_ioremap_resource() to make the whole 
> thing simpler.
Yes, I was going to simplify the code here, but status check is still 
required.

So I'm waiting.

Thanks,

Tang Bin

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

On 2020/4/16 18:05, Robin Murphy wrote:
> On 2020-04-02 7:33 am, Tang Bin wrote:
>> Release resources when exiting on error.
>>
>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>> ---
>>   drivers/iommu/qcom_iommu.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
>> index 4328da0b0..c08aa9651 100644
>> --- a/drivers/iommu/qcom_iommu.c
>> +++ b/drivers/iommu/qcom_iommu.c
>> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct 
>> platform_device *pdev)
>>       qcom_iommu->dev = dev;
>>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    if (res)
>> +    if (res) {
>>           qcom_iommu->local_base = devm_ioremap_resource(dev, res);
>> +        if (IS_ERR(qcom_iommu->local_base))
>> +            return PTR_ERR(qcom_iommu->local_base);
>> +    }
>
> ...or just use devm_platform_ioremap_resource() to make the whole 
> thing simpler.
Yes, I was going to simplify the code here, but status check is still 
required.

So I'm waiting.

Thanks,

Tang Bin



_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/qcom:fix local_base status check

[Robin Murphy]

On 2020-04-02 7:33 am, Tang Bin wrote:
> Release resources when exiting on error.
> 
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>   drivers/iommu/qcom_iommu.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0..c08aa9651 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>   	qcom_iommu->dev = dev;
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (res)
> +	if (res) {
>   		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(qcom_iommu->local_base))
> +			return PTR_ERR(qcom_iommu->local_base);
> +	}

...or just use devm_platform_ioremap_resource() to make the whole thing 
simpler.

Robin.

>   
>   	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>   	if (IS_ERR(qcom_iommu->iface_clk)) {
> 

Re: [PATCH] iommu/qcom:fix local_base status check

[Robin Murphy]

On 2020-04-02 7:33 am, Tang Bin wrote:
> Release resources when exiting on error.
> 
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>   drivers/iommu/qcom_iommu.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0..c08aa9651 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>   	qcom_iommu->dev = dev;
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (res)
> +	if (res) {
>   		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(qcom_iommu->local_base))
> +			return PTR_ERR(qcom_iommu->local_base);
> +	}

...or just use devm_platform_ioremap_resource() to make the whole thing 
simpler.

Robin.

>   
>   	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>   	if (IS_ERR(qcom_iommu->iface_clk)) {
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

Hi Bjorn:

On 2020/4/2 14:45, Bjorn Andersson wrote:
> On Wed 01 Apr 23:33 PDT 2020, Tang Bin wrote:
>
>> Release resources when exiting on error.
>>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Thanks for your positive feedback.

I don't know whether the commit message affect this patch's result. If 
so, I think the commit message need more clarification. As follwos:

         The function qcom_iommu_device_probe() does not perform 
sufficient error checking after executing devm_ioremap_resource(), which 
can result in crashes if a critical error path is encountered.

Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")


I'm waiting for your reply actively.

Thanks,

Tang Bin


>
>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>> ---
>>   drivers/iommu/qcom_iommu.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
>> index 4328da0b0..c08aa9651 100644
>> --- a/drivers/iommu/qcom_iommu.c
>> +++ b/drivers/iommu/qcom_iommu.c
>> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>>   	qcom_iommu->dev = dev;
>>   
>>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	if (res)
>> +	if (res) {
>>   		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
>> +		if (IS_ERR(qcom_iommu->local_base))
>> +			return PTR_ERR(qcom_iommu->local_base);
>> +	}
>>   
>>   	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>>   	if (IS_ERR(qcom_iommu->iface_clk)) {
>> -- 
>> 2.20.1.windows.1
>>
>>
>>

Re: [PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

Hi Bjorn:

On 2020/4/2 14:45, Bjorn Andersson wrote:
> On Wed 01 Apr 23:33 PDT 2020, Tang Bin wrote:
>
>> Release resources when exiting on error.
>>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Thanks for your positive feedback.

I don't know whether the commit message affect this patch's result. If 
so, I think the commit message need more clarification. As follwos:

         The function qcom_iommu_device_probe() does not perform 
sufficient error checking after executing devm_ioremap_resource(), which 
can result in crashes if a critical error path is encountered.

Fixes: 0ae349a0("iommu/qcom: Add qcom_iommu")


I'm waiting for your reply actively.

Thanks,

Tang Bin


>
>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>> ---
>>   drivers/iommu/qcom_iommu.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
>> index 4328da0b0..c08aa9651 100644
>> --- a/drivers/iommu/qcom_iommu.c
>> +++ b/drivers/iommu/qcom_iommu.c
>> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>>   	qcom_iommu->dev = dev;
>>   
>>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	if (res)
>> +	if (res) {
>>   		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
>> +		if (IS_ERR(qcom_iommu->local_base))
>> +			return PTR_ERR(qcom_iommu->local_base);
>> +	}
>>   
>>   	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>>   	if (IS_ERR(qcom_iommu->iface_clk)) {
>> -- 
>> 2.20.1.windows.1
>>
>>
>>


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/qcom:fix local_base status check

[Bjorn Andersson]

On Wed 01 Apr 23:33 PDT 2020, Tang Bin wrote:

> Release resources when exiting on error.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>  drivers/iommu/qcom_iommu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0..c08aa9651 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  	qcom_iommu->dev = dev;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (res)
> +	if (res) {
>  		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(qcom_iommu->local_base))
> +			return PTR_ERR(qcom_iommu->local_base);
> +	}
>  
>  	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>  	if (IS_ERR(qcom_iommu->iface_clk)) {
> -- 
> 2.20.1.windows.1
> 
> 
> 

Re: [PATCH] iommu/qcom:fix local_base status check

[Bjorn Andersson]

On Wed 01 Apr 23:33 PDT 2020, Tang Bin wrote:

> Release resources when exiting on error.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>  drivers/iommu/qcom_iommu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0..c08aa9651 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  	qcom_iommu->dev = dev;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (res)
> +	if (res) {
>  		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(qcom_iommu->local_base))
> +			return PTR_ERR(qcom_iommu->local_base);
> +	}
>  
>  	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
>  	if (IS_ERR(qcom_iommu->iface_clk)) {
> -- 
> 2.20.1.windows.1
> 
> 
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

Release resources when exiting on error.

Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 drivers/iommu/qcom_iommu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 4328da0b0..c08aa9651 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 	qcom_iommu->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res)
+	if (res) {
 		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(qcom_iommu->local_base))
+			return PTR_ERR(qcom_iommu->local_base);
+	}
 
 	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
 	if (IS_ERR(qcom_iommu->iface_clk)) {
-- 
2.20.1.windows.1

[PATCH] iommu/qcom:fix local_base status check

[Tang Bin]

Release resources when exiting on error.

Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 drivers/iommu/qcom_iommu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 4328da0b0..c08aa9651 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 	qcom_iommu->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res)
+	if (res) {
 		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(qcom_iommu->local_base))
+			return PTR_ERR(qcom_iommu->local_base);
+	}
 
 	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
 	if (IS_ERR(qcom_iommu->iface_clk)) {
-- 
2.20.1.windows.1



_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu