Re: [PATCH] soc: fsl: dpio: Change 'cpumask_t mask' to global variable

[Laurentiu Tudor]

Hi,

Thanks for finding this. Comment inline.

On 10/14/2020 10:27 AM, Yi Wang wrote:
> 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 global variable.
> 
> Signed-off-by: Hao Si <si.hao@xxxxxxxxxx>
> Signed-off-by: Lin Chen <chen.lin5@xxxxxxxxxx>
> Signed-off-by: Yi Wang <wang.yi59@xxxxxxxxxx>
> ---
>  drivers/soc/fsl/dpio/dpio-driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c b/drivers/soc/fsl/dpio/dpio-driver.c
> index 7b642c3..b31ec53 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -31,6 +31,7 @@ struct dpio_priv {
>  	struct dpaa2_io *io;
>  };
>  
> +static cpumask_t mask;

There can be multiple dpio devices with their associated driver
instances so it's not ok to make the variable global. Please place it in
the driver's private data and while at it, please rename it to cpu_mask.

---
Thanks & Best Regards, Laurentiu