drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
functions, the size and granule passed in must be judged.
It must be ensured that the passed in parameter is not 0 and
the size is an integer multiple of the granule, otherwise it
will cause an infinite while loop.
This was encountered during testing, and was initially triggered
by passing in a size value of 0, causing the kernel to crash.
[ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered, assigned bus number 1
[ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
[ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP) idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
[ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
[ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted 6.6.0-25.02.2500.002.uos25.aarch64 #1
[ 68.249237][ C0] Hardware name: Inspur CS5260F /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
[ 68.250029][ C0] Workqueue: events work_for_cpu_fn
[ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
[ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
[ 68.252189][ C0] sp : ffff80008044b780
[ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000 x27: 0000000000001000
[ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001 x24: 0000000000000600
[ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000 x21: fffffc64e2e59000
[ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280 x18: 0000000000000000
[ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008 x15: 0000000000000000
[ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb x12: fffffcfba30e3880
[ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9 : ffffb0e7decd1b5c
[ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6 : ffff3ee8c4148000
[ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3 : ffff3fe944c40800
[ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0 : ffffb0e7dfd6c3d0
[ 68.259185][ C0] Call trace:
[ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
[ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
[ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
[ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
[ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
[ 68.261654][ C0] __iommu_map+0x134/0x2a8
[ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
[ 68.262500][ C0] __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
[ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
[ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
[ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
[ 68.264372][ C0] xhci_init+0x88/0x1d0
[ 68.264736][ C0] xhci_gen_setup+0x284/0x468
[ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
[ 68.265506][ C0] usb_add_hcd+0x278/0x650
[ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
[ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
[ 68.266660][ C0] local_pci_probe+0x48/0xb8
[ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
[ 68.267548][ C0] process_one_work+0x170/0x3c0
[ 68.267999][ C0] worker_thread+0x234/0x3b8
[ 68.268383][ C0] kthread+0xf0/0x108
[ 68.268704][ C0] ret_from_fork+0x10/0x20
Signed-off-by: goutongchen <goutongchen@uniontech.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 8321962b3714..16b2e9ec0e60 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
int idx = cfg->cbndx;
+ if (size == 0 || granule == 0 || (size % granule) != 0) {
+ dev_err(smmu->dev,
+ "The size or granule passed in is err. size=%lu, granule=%lu\n",
+ size, granule);
+ return;
+ }
+
if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
wmb();
@@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long iova, size_t size,
struct arm_smmu_device *smmu = smmu_domain->smmu;
int idx = smmu_domain->cfg.cbndx;
+ if (size == 0 || granule == 0 || (size % granule) != 0) {
+ dev_err(smmu->dev,
+ "The size or granule passed in is err. size=%lu, granule=%lu\n",
+ size, granule);
+ return;
+ }
+
if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
wmb();
--
2.20.1Hi goutongchen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on soc/for-next]
[also build test WARNING on arm-perf/for-next/perf linus/master joro-iommu/next v6.12-rc4 next-20241024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/goutongchen/iommu-arm-smmu-Add-judgment-on-the-size-and-granule-parameters-passed-in/20241024-181048
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20241024100224.62942-1-goutongchen%40uniontech.com
patch subject: [PATCH] iommu/arm-smmu: Add judgment on the size and granule parameters passed in
config: i386-buildonly-randconfig-005-20241025 (https://download.01.org/0day-ci/archive/20241025/202410251314.KllYat7L-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241025/202410251314.KllYat7L-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410251314.KllYat7L-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/acpi.h:14,
from drivers/iommu/arm/arm-smmu/arm-smmu.c:20:
drivers/iommu/arm/arm-smmu/arm-smmu.c: In function 'arm_smmu_tlb_inv_range_s1':
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:287:34: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:154:56: note: in expansion of macro 'dev_fmt'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:286:17: note: in expansion of macro 'dev_err'
286 | dev_err(smmu->dev,
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:287:80: note: format string is defined here
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~^
| |
| long unsigned int
| %u
drivers/iommu/arm/arm-smmu/arm-smmu.c:287:34: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:154:56: note: in expansion of macro 'dev_fmt'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:286:17: note: in expansion of macro 'dev_err'
286 | dev_err(smmu->dev,
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:287:93: note: format string is defined here
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~^
| |
| long unsigned int
| %u
drivers/iommu/arm/arm-smmu/arm-smmu.c: In function 'arm_smmu_tlb_inv_range_s2':
drivers/iommu/arm/arm-smmu/arm-smmu.c:321:34: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:154:56: note: in expansion of macro 'dev_fmt'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:320:17: note: in expansion of macro 'dev_err'
320 | dev_err(smmu->dev,
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:321:80: note: format string is defined here
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~^
| |
| long unsigned int
| %u
drivers/iommu/arm/arm-smmu/arm-smmu.c:321:34: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:154:56: note: in expansion of macro 'dev_fmt'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:320:17: note: in expansion of macro 'dev_err'
320 | dev_err(smmu->dev,
| ^~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:321:93: note: format string is defined here
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~^
| |
| long unsigned int
| %u
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
vim +287 drivers/iommu/arm/arm-smmu/arm-smmu.c
276
277 static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
278 size_t granule, void *cookie, int reg)
279 {
280 struct arm_smmu_domain *smmu_domain = cookie;
281 struct arm_smmu_device *smmu = smmu_domain->smmu;
282 struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
283 int idx = cfg->cbndx;
284
285 if (size == 0 || granule == 0 || (size % granule) != 0) {
286 dev_err(smmu->dev,
> 287 "The size or granule passed in is err. size=%lu, granule=%lu\n",
288 size, granule);
289 return;
290 }
291
292 if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
293 wmb();
294
295 if (cfg->fmt != ARM_SMMU_CTX_FMT_AARCH64) {
296 iova = (iova >> 12) << 12;
297 iova |= cfg->asid;
298 do {
299 arm_smmu_cb_write(smmu, idx, reg, iova);
300 iova += granule;
301 } while (size -= granule);
302 } else {
303 iova >>= 12;
304 iova |= (u64)cfg->asid << 48;
305 do {
306 arm_smmu_cb_writeq(smmu, idx, reg, iova);
307 iova += granule >> 12;
308 } while (size -= granule);
309 }
310 }
311
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi goutongchen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on soc/for-next]
[also build test WARNING on arm-perf/for-next/perf linus/master joro-iommu/next v6.12-rc4 next-20241024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/goutongchen/iommu-arm-smmu-Add-judgment-on-the-size-and-granule-parameters-passed-in/20241024-181048
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20241024100224.62942-1-goutongchen%40uniontech.com
patch subject: [PATCH] iommu/arm-smmu: Add judgment on the size and granule parameters passed in
config: i386-buildonly-randconfig-003-20241025 (https://download.01.org/0day-ci/archive/20241025/202410251339.i3YUd5BO-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241025/202410251339.i3YUd5BO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410251339.i3YUd5BO-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/iommu/arm/arm-smmu/arm-smmu.c:24:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:288:6: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~~
| %zu
288 | size, granule);
| ^~~~
include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:288:12: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
287 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~~
| %zu
288 | size, granule);
| ^~~~~~~
include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:322:6: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~~
| %zu
322 | size, granule);
| ^~~~
include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
drivers/iommu/arm/arm-smmu/arm-smmu.c:322:12: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
321 | "The size or granule passed in is err. size=%lu, granule=%lu\n",
| ~~~
| %zu
322 | size, granule);
| ^~~~~~~
include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
5 warnings generated.
vim +288 drivers/iommu/arm/arm-smmu/arm-smmu.c
276
277 static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
278 size_t granule, void *cookie, int reg)
279 {
280 struct arm_smmu_domain *smmu_domain = cookie;
281 struct arm_smmu_device *smmu = smmu_domain->smmu;
282 struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
283 int idx = cfg->cbndx;
284
285 if (size == 0 || granule == 0 || (size % granule) != 0) {
286 dev_err(smmu->dev,
287 "The size or granule passed in is err. size=%lu, granule=%lu\n",
> 288 size, granule);
289 return;
290 }
291
292 if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
293 wmb();
294
295 if (cfg->fmt != ARM_SMMU_CTX_FMT_AARCH64) {
296 iova = (iova >> 12) << 12;
297 iova |= cfg->asid;
298 do {
299 arm_smmu_cb_write(smmu, idx, reg, iova);
300 iova += granule;
301 } while (size -= granule);
302 } else {
303 iova >>= 12;
304 iova |= (u64)cfg->asid << 48;
305 do {
306 arm_smmu_cb_writeq(smmu, idx, reg, iova);
307 iova += granule >> 12;
308 } while (size -= granule);
309 }
310 }
311
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 24/10/2024 11:02 am, goutongchen wrote:
> In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
> functions, the size and granule passed in must be judged.
> It must be ensured that the passed in parameter is not 0 and
> the size is an integer multiple of the granule, otherwise it
> will cause an infinite while loop.
>
> This was encountered during testing, and was initially triggered
> by passing in a size value of 0, causing the kernel to crash.
>
> [ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered, assigned bus number 1
> [ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
> [ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP) idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
> [ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
> [ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted 6.6.0-25.02.2500.002.uos25.aarch64 #1
> [ 68.249237][ C0] Hardware name: Inspur CS5260F /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
> [ 68.250029][ C0] Workqueue: events work_for_cpu_fn
> [ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
> [ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
> [ 68.252189][ C0] sp : ffff80008044b780
> [ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000 x27: 0000000000001000
> [ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001 x24: 0000000000000600
> [ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000 x21: fffffc64e2e59000
> [ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280 x18: 0000000000000000
> [ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008 x15: 0000000000000000
> [ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb x12: fffffcfba30e3880
> [ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9 : ffffb0e7decd1b5c
> [ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6 : ffff3ee8c4148000
> [ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3 : ffff3fe944c40800
> [ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0 : ffffb0e7dfd6c3d0
> [ 68.259185][ C0] Call trace:
> [ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
> [ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
> [ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
Huh? This invalidation path is for mapping a block entry, and the size
is the size of that block per ARM_LPAE_BLOCK_SIZE(lvl, data) - how can
it possibly be 0?
Thanks,
Robin.
> [ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
> [ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
> [ 68.261654][ C0] __iommu_map+0x134/0x2a8
> [ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
> [ 68.262500][ C0] __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
> [ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
> [ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
> [ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
> [ 68.264372][ C0] xhci_init+0x88/0x1d0
> [ 68.264736][ C0] xhci_gen_setup+0x284/0x468
> [ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
> [ 68.265506][ C0] usb_add_hcd+0x278/0x650
> [ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
> [ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
> [ 68.266660][ C0] local_pci_probe+0x48/0xb8
> [ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
> [ 68.267548][ C0] process_one_work+0x170/0x3c0
> [ 68.267999][ C0] worker_thread+0x234/0x3b8
> [ 68.268383][ C0] kthread+0xf0/0x108
> [ 68.268704][ C0] ret_from_fork+0x10/0x20
>
> Signed-off-by: goutongchen <goutongchen@uniontech.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 8321962b3714..16b2e9ec0e60 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> int idx = cfg->cbndx;
>
> + if (size == 0 || granule == 0 || (size % granule) != 0) {
> + dev_err(smmu->dev,
> + "The size or granule passed in is err. size=%lu, granule=%lu\n",
> + size, granule);
> + return;
> + }
> +
> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
> wmb();
>
> @@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long iova, size_t size,
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> int idx = smmu_domain->cfg.cbndx;
>
> + if (size == 0 || granule == 0 || (size % granule) != 0) {
> + dev_err(smmu->dev,
> + "The size or granule passed in is err. size=%lu, granule=%lu\n",
> + size, granule);
> + return;
> + }
> +
> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
> wmb();
>
在 2024/10/24 19:59, Robin Murphy 写道:
> On 24/10/2024 11:02 am, goutongchen wrote:
>> In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
>> functions, the size and granule passed in must be judged.
>> It must be ensured that the passed in parameter is not 0 and
>> the size is an integer multiple of the granule, otherwise it
>> will cause an infinite while loop.
>>
>> This was encountered during testing, and was initially triggered
>> by passing in a size value of 0, causing the kernel to crash.
>>
>> [ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered,
>> assigned bus number 1
>> [ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
>> [ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP)
>> idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
>> [ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
>> [ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted
>> 6.6.0-25.02.2500.002.uos25.aarch64 #1
>> [ 68.249237][ C0] Hardware name: Inspur CS5260F
>> /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
>> [ 68.250029][ C0] Workqueue: events work_for_cpu_fn
>> [ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO
>> -DIT -SSBS BTYPE=--)
>> [ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
>> [ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
>> [ 68.252189][ C0] sp : ffff80008044b780
>> [ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000
>> x27: 0000000000001000
>> [ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001
>> x24: 0000000000000600
>> [ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000
>> x21: fffffc64e2e59000
>> [ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280
>> x18: 0000000000000000
>> [ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008
>> x15: 0000000000000000
>> [ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb
>> x12: fffffcfba30e3880
>> [ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9
>> : ffffb0e7decd1b5c
>> [ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6
>> : ffff3ee8c4148000
>> [ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3
>> : ffff3fe944c40800
>> [ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0
>> : ffffb0e7dfd6c3d0
>> [ 68.259185][ C0] Call trace:
>> [ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
>> [ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
>> [ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
>
> Huh? This invalidation path is for mapping a block entry, and the size
> is the size of that block per ARM_LPAE_BLOCK_SIZE(lvl, data) - how can
> it possibly be 0?
>
> Thanks,
> Robin.
>
Thank you for your reply. First of all, as you said, under normal
circumstances, the size will not be 0, but I think it is better to limit
it here.
Because there is no guarantee that the parameter value passed in meets
the requirements, if a value that does not meet the requirements is set
manually during the call, it will fall into an infinite loop and cause
the kernel to fail to start, which is very fatal.
It is precisely because we actually encountered such an error and this
problem that we submitted this patch to the upstream.
Thanks,
goutongchen.
>> [ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
>> [ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
>> [ 68.261654][ C0] __iommu_map+0x134/0x2a8
>> [ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
>> [ 68.262500][ C0]
>> __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
>> [ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
>> [ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
>> [ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
>> [ 68.264372][ C0] xhci_init+0x88/0x1d0
>> [ 68.264736][ C0] xhci_gen_setup+0x284/0x468
>> [ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
>> [ 68.265506][ C0] usb_add_hcd+0x278/0x650
>> [ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
>> [ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
>> [ 68.266660][ C0] local_pci_probe+0x48/0xb8
>> [ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
>> [ 68.267548][ C0] process_one_work+0x170/0x3c0
>> [ 68.267999][ C0] worker_thread+0x234/0x3b8
>> [ 68.268383][ C0] kthread+0xf0/0x108
>> [ 68.268704][ C0] ret_from_fork+0x10/0x20
>>
>> Signed-off-by: goutongchen <goutongchen@uniontech.com>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> index 8321962b3714..16b2e9ec0e60 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned
>> long iova, size_t size,
>> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>> int idx = cfg->cbndx;
>> + if (size == 0 || granule == 0 || (size % granule) != 0) {
>> + dev_err(smmu->dev,
>> + "The size or granule passed in is err. size=%lu,
>> granule=%lu\n",
>> + size, granule);
>> + return;
>> + }
>> +
>> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
>> wmb();
>> @@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned
>> long iova, size_t size,
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> int idx = smmu_domain->cfg.cbndx;
>> + if (size == 0 || granule == 0 || (size % granule) != 0) {
>> + dev_err(smmu->dev,
>> + "The size or granule passed in is err. size=%lu,
>> granule=%lu\n",
>> + size, granule);
>> + return;
>> + }
>> +
>> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
>> wmb();
>
From: goutongchen <goutongchen@uniontech.com>
In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
functions, the size and granule passed in must be judged.
It must be ensured that the passed in parameter is not 0 and
the size is an integer multiple of the granule, otherwise it
will cause an infinite while loop.
This was encountered during testing, and was initially triggered
by passing in a size value of 0, causing the kernel to crash.
[ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered, assigned bus number 1
[ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
[ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP) idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
[ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
[ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted 6.6.0-25.02.2500.002.uos25.aarch64 #1
[ 68.249237][ C0] Hardware name: Inspur CS5260F /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
[ 68.250029][ C0] Workqueue: events work_for_cpu_fn
[ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
[ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
[ 68.252189][ C0] sp : ffff80008044b780
[ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000 x27: 0000000000001000
[ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001 x24: 0000000000000600
[ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000 x21: fffffc64e2e59000
[ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280 x18: 0000000000000000
[ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008 x15: 0000000000000000
[ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb x12: fffffcfba30e3880
[ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9 : ffffb0e7decd1b5c
[ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6 : ffff3ee8c4148000
[ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3 : ffff3fe944c40800
[ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0 : ffffb0e7dfd6c3d0
[ 68.259185][ C0] Call trace:
[ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
[ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
[ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
[ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
[ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
[ 68.261654][ C0] __iommu_map+0x134/0x2a8
[ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
[ 68.262500][ C0] __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
[ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
[ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
[ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
[ 68.264372][ C0] xhci_init+0x88/0x1d0
[ 68.264736][ C0] xhci_gen_setup+0x284/0x468
[ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
[ 68.265506][ C0] usb_add_hcd+0x278/0x650
[ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
[ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
[ 68.266660][ C0] local_pci_probe+0x48/0xb8
[ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
[ 68.267548][ C0] process_one_work+0x170/0x3c0
[ 68.267999][ C0] worker_thread+0x234/0x3b8
[ 68.268383][ C0] kthread+0xf0/0x108
[ 68.268704][ C0] ret_from_fork+0x10/0x20
Signed-off-by: goutongchen <goutongchen@uniontech.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 8321962b3714..fdd7d7e9ce06 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
int idx = cfg->cbndx;
+ if (size == 0 || granule == 0 || (size % granule) != 0) {
+ dev_err(smmu->dev,
+ "The size or granule passed in is err. size=%zu, granule=%zu\n",
+ size, granule);
+ return;
+ }
+
if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
wmb();
@@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long iova, size_t size,
struct arm_smmu_device *smmu = smmu_domain->smmu;
int idx = smmu_domain->cfg.cbndx;
+ if (size == 0 || granule == 0 || (size % granule) != 0) {
+ dev_err(smmu->dev,
+ "The size or granule passed in is err. size=%zu, granule=%zu\n",
+ size, granule);
+ return;
+ }
+
if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
wmb();
--
2.20.1On 2024-10-28 3:48 am, goutongchen@uniontech.com wrote:
> From: goutongchen <goutongchen@uniontech.com>
>
> In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
> functions, the size and granule passed in must be judged.
> It must be ensured that the passed in parameter is not 0 and
> the size is an integer multiple of the granule, otherwise it
> will cause an infinite while loop.
>
> This was encountered during testing, and was initially triggered
> by passing in a size value of 0, causing the kernel to crash.
Still NAK. If there is a bug in the upstream io-pgtable-arm code which
can actually cause this, please send a patch to fix *that* bug.
Otherwise, if you've made a broken downstream change then it is not
upstream's responsibility to maintain unnecessary code in a questionable
attempt to paper over (some small subset of) such brokenness.
If you pass any sufficiently large size value which *does* happen to be
a multiple of the granule, you're still going to see the same RCU stalls
and failure to progress within reasonable time. If you pass something
inappropriate for the "cookie" pointer, you're likely to corrupt memory
or really crash. If you pass arguments which all look plausible but
still don't match what actually needs invalidating, the consequences of
under-invalidation can be even more subtle, nasty and hard to debug.
The iommu_flush_ops are not a public interface intended to be called
arbitrarily from anywhere in the kernel with unchecked input, they are a
low-level private interface between IOMMU drivers and their respective
io-pgtable implementations, and as such they are designed for their
callers to call them correctly by construction. Calling them incorrectly
indicates a serious bug in the caller, since getting mapping and/or TLB
invalidation wrong often leads to memory corruption or other issues down
the line. Hence I'm not convinced this change is actually even desirable
as a downstream debugging aid - if you're lucky enough to get stuck on
an obviously-wrong call here, that's surely the clearest possible
indication of the source of the bug in its calling context, far better
than trying to ignore it and then having it drowned out by more distant
things blowing up later due to 2nd- and 3rd-order effects.
Thanks,
Robin.
> [ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered, assigned bus number 1
> [ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
> [ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP) idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
> [ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
> [ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted 6.6.0-25.02.2500.002.uos25.aarch64 #1
> [ 68.249237][ C0] Hardware name: Inspur CS5260F /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
> [ 68.250029][ C0] Workqueue: events work_for_cpu_fn
> [ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
> [ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
> [ 68.252189][ C0] sp : ffff80008044b780
> [ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000 x27: 0000000000001000
> [ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001 x24: 0000000000000600
> [ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000 x21: fffffc64e2e59000
> [ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280 x18: 0000000000000000
> [ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008 x15: 0000000000000000
> [ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb x12: fffffcfba30e3880
> [ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9 : ffffb0e7decd1b5c
> [ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6 : ffff3ee8c4148000
> [ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3 : ffff3fe944c40800
> [ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0 : ffffb0e7dfd6c3d0
> [ 68.259185][ C0] Call trace:
> [ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
> [ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
> [ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
> [ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
> [ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
> [ 68.261654][ C0] __iommu_map+0x134/0x2a8
> [ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
> [ 68.262500][ C0] __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
> [ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
> [ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
> [ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
> [ 68.264372][ C0] xhci_init+0x88/0x1d0
> [ 68.264736][ C0] xhci_gen_setup+0x284/0x468
> [ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
> [ 68.265506][ C0] usb_add_hcd+0x278/0x650
> [ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
> [ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
> [ 68.266660][ C0] local_pci_probe+0x48/0xb8
> [ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
> [ 68.267548][ C0] process_one_work+0x170/0x3c0
> [ 68.267999][ C0] worker_thread+0x234/0x3b8
> [ 68.268383][ C0] kthread+0xf0/0x108
> [ 68.268704][ C0] ret_from_fork+0x10/0x20
>
> Signed-off-by: goutongchen <goutongchen@uniontech.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 8321962b3714..fdd7d7e9ce06 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size,
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> int idx = cfg->cbndx;
>
> + if (size == 0 || granule == 0 || (size % granule) != 0) {
> + dev_err(smmu->dev,
> + "The size or granule passed in is err. size=%zu, granule=%zu\n",
> + size, granule);
> + return;
> + }
> +
> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
> wmb();
>
> @@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long iova, size_t size,
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> int idx = smmu_domain->cfg.cbndx;
>
> + if (size == 0 || granule == 0 || (size % granule) != 0) {
> + dev_err(smmu->dev,
> + "The size or granule passed in is err. size=%zu, granule=%zu\n",
> + size, granule);
> + return;
> + }
> +
> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
> wmb();
>
在 2024/10/28 19:41, Robin Murphy 写道:
> On 2024-10-28 3:48 am, goutongchen@uniontech.com wrote:
>> From: goutongchen <goutongchen@uniontech.com>
>>
>> In the arm_smmu_tlb_inv_range_s1 and arm_smmu_tlb_inv_range_s2
>> functions, the size and granule passed in must be judged.
>> It must be ensured that the passed in parameter is not 0 and
>> the size is an integer multiple of the granule, otherwise it
>> will cause an infinite while loop.
>>
>> This was encountered during testing, and was initially triggered
>> by passing in a size value of 0, causing the kernel to crash.
>
> Still NAK. If there is a bug in the upstream io-pgtable-arm code which
> can actually cause this, please send a patch to fix *that* bug.
> Otherwise, if you've made a broken downstream change then it is not
> upstream's responsibility to maintain unnecessary code in a
> questionable attempt to paper over (some small subset of) such
> brokenness.
>
> If you pass any sufficiently large size value which *does* happen to
> be a multiple of the granule, you're still going to see the same RCU
> stalls and failure to progress within reasonable time. If you pass
> something inappropriate for the "cookie" pointer, you're likely to
> corrupt memory or really crash. If you pass arguments which all look
> plausible but still don't match what actually needs invalidating, the
> consequences of under-invalidation can be even more subtle, nasty and
> hard to debug.
>
> The iommu_flush_ops are not a public interface intended to be called
> arbitrarily from anywhere in the kernel with unchecked input, they are
> a low-level private interface between IOMMU drivers and their
> respective io-pgtable implementations, and as such they are designed
> for their callers to call them correctly by construction. Calling them
> incorrectly indicates a serious bug in the caller, since getting
> mapping and/or TLB invalidation wrong often leads to memory corruption
> or other issues down the line. Hence I'm not convinced this change is
> actually even desirable as a downstream debugging aid - if you're
> lucky enough to get stuck on an obviously-wrong call here, that's
> surely the clearest possible indication of the source of the bug in
> its calling context, far better than trying to ignore it and then
> having it drowned out by more distant things blowing up later due to
> 2nd- and 3rd-order effects.
>
> Thanks,
> Robin.
>
OK! Thanks, very much!
Goutongchen.
>> [ 8.214378][ T675] xhci_hcd 0000:0b:00.0: new USB bus registered,
>> assigned bus number 1
>> [ 68.246185][ C0] rcu: INFO: rcu_sched self-detected stall on CPU
>> [ 68.246866][ C0] rcu: 0-....: (5999 ticks this GP)
>> idle=796c/1/0x4000000000000000 softirq=161/161 fqs=2999
>> [ 68.247924][ C0] rcu: (t=6000 jiffies g=-699 q=1 ncpus=128)
>> [ 68.248452][ C0] CPU: 0 PID: 675 Comm: kworker/0:2 Not tainted
>> 6.6.0-25.02.2500.002.uos25.aarch64 #1
>> [ 68.249237][ C0] Hardware name: Inspur CS5260F
>> /CS5260F , BIOS 4.0.16 05/31/22 16:53:51
>> [ 68.250029][ C0] Workqueue: events work_for_cpu_fn
>> [ 68.250497][ C0] pstate: a0000005 (NzCv daif -PAN -UAO -TCO
>> -DIT -SSBS BTYPE=--)
>> [ 68.251188][ C0] pc : arm_smmu_tlb_inv_range_s1+0xf0/0x158
>> [ 68.251704][ C0] lr : arm_smmu_tlb_inv_walk_s1+0x44/0x68
>> [ 68.252189][ C0] sp : ffff80008044b780
>> [ 68.252530][ C0] x29: ffff80008044b780 x28: 0000000000000000
>> x27: 0000000000001000
>> [ 68.253212][ C0] x26: 0000000000000600 x25: 0000000000000001
>> x24: 0000000000000600
>> [ 68.253857][ C0] x23: 0000000000000000 x22: 0000000000001000
>> x21: fffffc64e2e59000
>> [ 68.254544][ C0] x20: 0000000039c1d1a6 x19: ffff3fe944c40280
>> x18: 0000000000000000
>> [ 68.255240][ C0] x17: 626d756e20737562 x16: ffffb0e7e08c1008
>> x15: 0000000000000000
>> [ 68.255930][ C0] x14: ffff3ef8c1ea15cd x13: ffff3ef8c1ea15cb
>> x12: fffffcfba30e3880
>> [ 68.256538][ C0] x11: 00000000ffff7fff x10: ffff80008044b6b0 x9
>> : ffffb0e7decd1b5c
>> [ 68.257126][ C0] x8 : 0000000000000dc0 x7 : ffff3ee8c4148000 x6
>> : ffff3ee8c4148000
>> [ 68.257822][ C0] x5 : 0000000000000008 x4 : 0000000000000000 x3
>> : ffff3fe944c40800
>> [ 68.258497][ C0] x2 : 0000000000000010 x1 : 0000000000000020 x0
>> : ffffb0e7dfd6c3d0
>> [ 68.259185][ C0] Call trace:
>> [ 68.259451][ C0] arm_smmu_tlb_inv_range_s1+0xf0/0x158
>> [ 68.259933][ C0] arm_smmu_tlb_inv_walk_s1+0x44/0x68
>> [ 68.260384][ C0] __arm_lpae_map+0x1f0/0x2c0
>> [ 68.260796][ C0] arm_lpae_map_pages+0xec/0x150
>> [ 68.261215][ C0] arm_smmu_map_pages+0x48/0x130
>> [ 68.261654][ C0] __iommu_map+0x134/0x2a8
>> [ 68.262098][ C0] iommu_map_sg+0xb8/0x1c8
>> [ 68.262500][ C0]
>> __iommu_dma_alloc_noncontiguous.constprop.0+0x180/0x270
>> [ 68.263145][ C0] iommu_dma_alloc+0x178/0x238
>> [ 68.263557][ C0] dma_alloc_attrs+0xf8/0x108
>> [ 68.263962][ C0] xhci_mem_init+0x1e8/0x6d0
>> [ 68.264372][ C0] xhci_init+0x88/0x1d0
>> [ 68.264736][ C0] xhci_gen_setup+0x284/0x468
>> [ 68.265121][ C0] xhci_pci_setup+0x60/0x1f8
>> [ 68.265506][ C0] usb_add_hcd+0x278/0x650
>> [ 68.265860][ C0] usb_hcd_pci_probe+0x218/0x458
>> [ 68.266256][ C0] xhci_pci_probe+0x7c/0x270
>> [ 68.266660][ C0] local_pci_probe+0x48/0xb8
>> [ 68.267074][ C0] work_for_cpu_fn+0x24/0x40
>> [ 68.267548][ C0] process_one_work+0x170/0x3c0
>> [ 68.267999][ C0] worker_thread+0x234/0x3b8
>> [ 68.268383][ C0] kthread+0xf0/0x108
>> [ 68.268704][ C0] ret_from_fork+0x10/0x20
>>
>> Signed-off-by: goutongchen <goutongchen@uniontech.com>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> index 8321962b3714..fdd7d7e9ce06 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -282,6 +282,13 @@ static void arm_smmu_tlb_inv_range_s1(unsigned
>> long iova, size_t size,
>> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>> int idx = cfg->cbndx;
>> + if (size == 0 || granule == 0 || (size % granule) != 0) {
>> + dev_err(smmu->dev,
>> + "The size or granule passed in is err. size=%zu,
>> granule=%zu\n",
>> + size, granule);
>> + return;
>> + }
>> +
>> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
>> wmb();
>> @@ -309,6 +316,13 @@ static void arm_smmu_tlb_inv_range_s2(unsigned
>> long iova, size_t size,
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> int idx = smmu_domain->cfg.cbndx;
>> + if (size == 0 || granule == 0 || (size % granule) != 0) {
>> + dev_err(smmu->dev,
>> + "The size or granule passed in is err. size=%zu,
>> granule=%zu\n",
>> + size, granule);
>> + return;
>> + }
>> +
>> if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
>> wmb();
>
>
© 2016 - 2024 Red Hat, Inc.