RE: [PATCH v2] soc: fsl: dpio: Change 'cpumask_t mask' to the driver's private data
From: Leo Li
Date: Fri Oct 16 2020 - 17:20:32 EST
> -----Original Message-----
> From: Yi Wang <wang.yi59@xxxxxxxxxx>
> Sent: Friday, October 16, 2020 1:49 AM
> To: Roy Pledge <roy.pledge@xxxxxxx>; Laurentiu Tudor
> <laurentiu.tudor@xxxxxxx>
> Cc: Leo Li <leoyang.li@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linuxppc-
> dev@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> xue.zhihong@xxxxxxxxxx; wang.yi59@xxxxxxxxxx; jiang.xuexin@xxxxxxxxxx;
> Hao Si <si.hao@xxxxxxxxxx>; Lin Chen <chen.lin5@xxxxxxxxxx>
> Subject: [PATCH v2] soc: fsl: dpio: Change 'cpumask_t mask' to the driver's
> private data
>
> From: Hao Si <si.hao@xxxxxxxxxx>
>
> The local variable 'cpumask_t mask' is in the stack memory, and its address
> is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
> But the memory area where this variable is located is at risk of being
> modified.
>
> During LTP testing, the following error was generated:
>
> Unable to handle kernel paging request at virtual address ffff000012e9b790
> Mem abort info:
> ESR = 0x96000007
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000007
> CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp = 0000000075ac5e07
> [ffff000012e9b790] pgd=00000027dbffe003, pud=00000027dbffd003,
> pmd=00000027b6d61003, pte=0000000000000000
> Internal error: Oops: 96000007 [#1] PREEMPT SMP
> Modules linked in: xt_conntrack
> Process read_all (pid: 20171, stack limit = 0x0000000044ea4095)
> CPU: 14 PID: 20171 Comm: read_all Tainted: G B W
> Hardware name: NXP Layerscape LX2160ARDB (DT)
> pstate: 80000085 (Nzcv daIf -PAN -UAO)
> pc : irq_affinity_hint_proc_show+0x54/0xb0
> lr : irq_affinity_hint_proc_show+0x4c/0xb0
> sp : ffff00001138bc10
> x29: ffff00001138bc10 x28: 0000ffffd131d1e0
> x27: 00000000007000c0 x26: ffff8025b9480dc0
> x25: ffff8025b9480da8 x24: 00000000000003ff
> x23: ffff8027334f8300 x22: ffff80272e97d000
> x21: ffff80272e97d0b0 x20: ffff8025b9480d80
> x19: ffff000009a49000 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000
> x15: 0000000000000000 x14: 0000000000000000
> x13: 0000000000000000 x12: 0000000000000040
> x11: 0000000000000000 x10: ffff802735b79b88
> x9 : 0000000000000000 x8 : 0000000000000000
> x7 : ffff000009a49848 x6 : 0000000000000003
> x5 : 0000000000000000 x4 : ffff000008157d6c
> x3 : ffff00001138bc10 x2 : ffff000012e9b790
> x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> irq_affinity_hint_proc_show+0x54/0xb0
> seq_read+0x1b0/0x440
> proc_reg_read+0x80/0xd8
> __vfs_read+0x60/0x178
> vfs_read+0x94/0x150
> ksys_read+0x74/0xf0
> __arm64_sys_read+0x24/0x30
> el0_svc_common.constprop.0+0xd8/0x1a0
> el0_svc_handler+0x34/0x88
> el0_svc+0x10/0x14
> Code: f9001bbf 943e0732 f94066c2 b4000062 (f9400041)
> ---[ end trace b495bdcb0b3b732b ]---
> Kernel panic - not syncing: Fatal exception
> SMP: stopping secondary CPUs
> SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
> Kernel Offset: disabled
> CPU features: 0x0,21006008
> Memory Limit: none
> ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Fix it by changing 'cpumask_t mask' to the driver's private data.
Thanks for the patch. Sorry to chime in late.
Since we are only setting single bit in the cpumask, it is actually not necessary to keep the cpumask in private data as we already kept the cpu number in desc.cpu. The better and easier approach is to actually use get_cpu_mask(cpu) API to get the pre-defined cpumask in the static cpu_bit_bitmap array. We don't even need to declare the mask/cpu_mask in the register_dpio_irq_handlers().
>
> Signed-off-by: Hao Si <si.hao@xxxxxxxxxx>
> Signed-off-by: Lin Chen <chen.lin5@xxxxxxxxxx>
> Signed-off-by: Yi Wang <wang.yi59@xxxxxxxxxx>
> ---
> v2: Place 'cpumask_t mask' in the driver's private data and while at it,
> rename it to cpu_mask.
>
> drivers/soc/fsl/dpio/dpio-driver.c | 9 +++++----
> include/linux/fsl/mc.h | 2 ++
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c b/drivers/soc/fsl/dpio/dpio-
> driver.c
> index 7b642c3..e9d820d 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -95,7 +95,7 @@ static int register_dpio_irq_handlers(struct
> fsl_mc_device *dpio_dev, int cpu)
> {
> int error;
> struct fsl_mc_device_irq *irq;
> - cpumask_t mask;
> + cpumask_t *cpu_mask;
>
> irq = dpio_dev->irqs[0];
> error = devm_request_irq(&dpio_dev->dev,
> @@ -112,9 +112,10 @@ static int register_dpio_irq_handlers(struct
> fsl_mc_device *dpio_dev, int cpu)
> }
>
> /* set the affinity hint */
> - cpumask_clear(&mask);
> - cpumask_set_cpu(cpu, &mask);
> - if (irq_set_affinity_hint(irq->msi_desc->irq, &mask))
> + cpu_mask = &dpio_dev->mask;
> + cpumask_clear(cpu_mask);
> + cpumask_set_cpu(cpu, cpu_mask);
> + if (irq_set_affinity_hint(irq->msi_desc->irq, cpu_mask))
> dev_err(&dpio_dev->dev,
> "irq_set_affinity failed irq %d cpu %d\n",
> irq->msi_desc->irq, cpu);
> diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
> index a428c61..ebdfb54 100644
> --- a/include/linux/fsl/mc.h
> +++ b/include/linux/fsl/mc.h
> @@ -151,6 +151,7 @@ struct fsl_mc_obj_desc {
> /**
> * struct fsl_mc_device - MC object device object
> * @dev: Linux driver model device object
> + * @mask: cpu mask for affinity_hint
> * @dma_mask: Default DMA mask
> * @flags: MC object device flags
> * @icid: Isolation context ID for the device
> @@ -184,6 +185,7 @@ struct fsl_mc_obj_desc {
> */
> struct fsl_mc_device {
> struct device dev;
> + cpumask_t mask;
> u64 dma_mask;
> u16 flags;
> u16 icid;
> --
> 2.15.2