Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain()
@ 2020-05-22 13:01 Sai Prakash Ranjan
  2020-05-25 13:02 ` Joerg Roedel
  0 siblings, 1 reply; 3+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-22 13:01 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Joerg Roedel
  Cc: iommu, linux-kernel, linux-arm-msm, linux-arm-kernel, Evan Green,
	Douglas Anderson, Sai Prakash Ranjan

Since the change to move default domain allocation to probe,
there is a refcount decrement missing for the group in
iommu_alloc_default_domain(). Because of this missing
refcount decrement, the device is never released from the
group as the devices_kobj refcount never reaches 0 in
iommu_group_remove_device() leading to a lot of issues.
One such case is that this will lead to a different group
allocation on every reload of the module which configures
iommu such as the ath10k module which then finally fails
to attach this device to the SMMU with -ENOSPC error in
__arm_smmu_alloc_bitmap() once the count of module reload
crosses the number of context banks. This will then lead
to NULL pointer deference in the next reload of the module.
Add the missing refcount decrement(iommu_group_put()) in
iommu_alloc_default_domain() to fix this issue.

Call trace:

<snip>...
  platform wifi-firmware.0: Adding to iommu group 82
  ath10k_snoc 18800000.wifi: could not attach device: -28
  platform wifi-firmware.0: Removing from iommu group 82
  ath10k_snoc 18800000.wifi: failed to initialize firmware: -28
  ath10k_snoc: probe of 18800000.wifi failed with error -28
  platform wifi-firmware.0: Adding to iommu group 83
  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
  Mem abort info:
    ESR = 0x96000006
    EC = 0x25: DABT (current EL), IL = 32 bits
    SET = 0, FnV = 0
    EA = 0, S1PTW = 0
  Data abort info:
    ISV = 0, ISS = 0x00000006
    CM = 0, WnR = 0
  user pgtable: 4k pages, 39-bit VAs, pgdp=0000000177a53000
  [0000000000000000] pgd=00000001e74f5003, pud=00000001e74f5003, pmd=0000000000000000
  Internal error: Oops: 96000006 [#1] PREEMPT SMP
  pstate: 60400009 (nZCv daif +PAN -UAO)
  arm_smmu_flush_iotlb_all+0x20/0x6c
  iommu_create_device_direct_mappings+0x17c/0x1d8
  iommu_probe_device+0xc0/0x100
  of_iommu_configure+0x108/0x240
  of_dma_configure+0x130/0x1d0
  ath10k_fw_init+0xc4/0x1c4 [ath10k_snoc]
  ath10k_snoc_probe+0x5cc/0x678 [ath10k_snoc]
  platform_drv_probe+0x90/0xb0
  really_probe+0x134/0x2ec
  driver_probe_device+0x64/0xfc
  device_driver_attach+0x4c/0x6c
  __driver_attach+0xac/0xc0
  bus_for_each_dev+0x8c/0xd4
  driver_attach+0x2c/0x38
  bus_add_driver+0xfc/0x1d0
  driver_register+0x64/0xf8
  __platform_driver_register+0x4c/0x58
  init_module+0x20/0x1000 [ath10k_snoc]
  do_one_initcall+0x13c/0x2d0
  do_init_module+0x58/0x1dc
  load_module+0xde0/0xf10
  __arm64_sys_finit_module+0xb0/0xe0
  el0_svc_common+0xa4/0x154
  el0_svc_compat_handler+0x2c/0x38
  el0_svc_compat+0x8/0x10
 Code: d503201f f85b8268 b4000248 f8560e74 (f9400280)
 ---[ end trace e5c1470a584952a0 ]---
 Kernel panic - not syncing: Fatal exception

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/iommu/iommu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a4c2f122eb8b..05f7b77c432f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1491,6 +1491,7 @@ static int iommu_alloc_default_domain(struct device *dev)
 {
 	struct iommu_group *group;
 	unsigned int type;
+	int ret;
 
 	group = iommu_group_get(dev);
 	if (!group)
@@ -1501,7 +1502,11 @@ static int iommu_alloc_default_domain(struct device *dev)
 
 	type = iommu_get_def_domain_type(dev);
 
-	return iommu_group_alloc_default_domain(dev->bus, group, type);
+	ret = iommu_group_alloc_default_domain(dev->bus, group, type);
+
+	iommu_group_put(group);
+
+	return ret;
 }
 
 /**
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain()
  2020-05-22 13:01 [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain() Sai Prakash Ranjan
@ 2020-05-25 13:02 ` Joerg Roedel
  2020-05-25 13:10   ` Sai Prakash Ranjan
  0 siblings, 1 reply; 3+ messages in thread
From: Joerg Roedel @ 2020-05-25 13:02 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Will Deacon, Robin Murphy, Joerg Roedel, iommu, linux-kernel,
	linux-arm-msm, linux-arm-kernel, Evan Green, Douglas Anderson

Hi,

On Fri, May 22, 2020 at 06:31:45PM +0530, Sai Prakash Ranjan wrote:
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a4c2f122eb8b..05f7b77c432f 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1491,6 +1491,7 @@ static int iommu_alloc_default_domain(struct device *dev)
>  {
>  	struct iommu_group *group;
>  	unsigned int type;
> +	int ret;
>  
>  	group = iommu_group_get(dev);
>  	if (!group)
> @@ -1501,7 +1502,11 @@ static int iommu_alloc_default_domain(struct device *dev)
>  
>  	type = iommu_get_def_domain_type(dev);
>  
> -	return iommu_group_alloc_default_domain(dev->bus, group, type);
> +	ret = iommu_group_alloc_default_domain(dev->bus, group, type);
> +
> +	iommu_group_put(group);
> +
> +	return ret;
>  }
>  
>  /**

Thanks for the report and the fix. I think it is better to fix this by
not taking a group reference in iommu_alloc_default_domain() at all and
pass group as a parameter. Please see the patch I just sent out.

Regards,

	Joerg

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain()
  2020-05-25 13:02 ` Joerg Roedel
@ 2020-05-25 13:10   ` Sai Prakash Ranjan
  0 siblings, 0 replies; 3+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-25 13:10 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Will Deacon, Joerg Roedel, linux-kernel, Evan Green,
	Douglas Anderson, iommu, linux-arm-msm, Robin Murphy,
	linux-arm-kernel

Hi Joerg,

On 2020-05-25 18:32, Joerg Roedel wrote:
> Hi,
> 
> On Fri, May 22, 2020 at 06:31:45PM +0530, Sai Prakash Ranjan wrote:
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index a4c2f122eb8b..05f7b77c432f 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1491,6 +1491,7 @@ static int iommu_alloc_default_domain(struct 
>> device *dev)
>>  {
>>  	struct iommu_group *group;
>>  	unsigned int type;
>> +	int ret;
>> 
>>  	group = iommu_group_get(dev);
>>  	if (!group)
>> @@ -1501,7 +1502,11 @@ static int iommu_alloc_default_domain(struct 
>> device *dev)
>> 
>>  	type = iommu_get_def_domain_type(dev);
>> 
>> -	return iommu_group_alloc_default_domain(dev->bus, group, type);
>> +	ret = iommu_group_alloc_default_domain(dev->bus, group, type);
>> +
>> +	iommu_group_put(group);
>> +
>> +	return ret;
>>  }
>> 
>>  /**
> 
> Thanks for the report and the fix. I think it is better to fix this by
> not taking a group reference in iommu_alloc_default_domain() at all and
> pass group as a parameter. Please see the patch I just sent out.
> 

Thanks for the patch, it looks like the right thing to do. Testing it
out now.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 13:01 [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain() Sai Prakash Ranjan
2020-05-25 13:02 ` Joerg Roedel
2020-05-25 13:10   ` Sai Prakash Ranjan

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git