* Re: [PATCH v2 34/37] KVM: PPC: Book3S HV: add virtual mode handlers for HPT hcalls and page faults
From: Nicholas Piggin @ 2021-03-04 11:05 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87im68vw16.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of March 4, 2021 6:09 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> In order to support hash guests in the P9 path (which does not do real
>> mode hcalls or page fault handling), these real-mode hash specific
>> interrupts need to be implemented in virt mode.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 118 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 113 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 9d2fa21201c1..1bbc46f2cfbf 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -935,6 +935,52 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>> return RESUME_HOST;
>>
>> switch (req) {
>> + case H_REMOVE:
>> + ret = kvmppc_h_remove(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_ENTER:
>> + ret = kvmppc_h_enter(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6),
>> + kvmppc_get_gpr(vcpu, 7));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_READ:
>> + ret = kvmppc_h_read(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_CLEAR_MOD:
>> + ret = kvmppc_h_clear_mod(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_CLEAR_REF:
>> + ret = kvmppc_h_clear_ref(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_PROTECT:
>> + ret = kvmppc_h_protect(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_BULK_REMOVE:
>> + ret = kvmppc_h_bulk_remove(vcpu);
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> +
>
> Some of these symbols need to be exported.
>
> ERROR: modpost: "kvmppc_h_bulk_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_clear_mod" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_xive_xics_hcall" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "decrementers_next_tb" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_hpte_hv_fault" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_protect" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_enter" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_clear_ref" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_read" [arch/powerpc/kvm/kvm-hv.ko] undefined!
Yeah sorry about that there's a few issues there, I'll try polish that
up a bit before the next post.
Thanks,
Nick
^ permalink raw reply
* Re: cleanup unused or almost unused IOMMU APIs and the FSL PAMU driver
From: Joerg Roedel @ 2021-03-04 11:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: freedreno, kvm, Will Deacon, dri-devel, Li Yang, iommu, netdev,
linux-arm-msm, virtualization, linuxppc-dev, David Woodhouse,
linux-arm-kernel, Lu Baolu
In-Reply-To: <20210301084257.945454-1-hch@lst.de>
On Mon, Mar 01, 2021 at 09:42:40AM +0100, Christoph Hellwig wrote:
> Diffstat:
> arch/powerpc/include/asm/fsl_pamu_stash.h | 12
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2
> drivers/iommu/amd/iommu.c | 23
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 85 ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 122 +---
> drivers/iommu/dma-iommu.c | 8
> drivers/iommu/fsl_pamu.c | 264 ----------
> drivers/iommu/fsl_pamu.h | 10
> drivers/iommu/fsl_pamu_domain.c | 694 ++--------------------------
> drivers/iommu/fsl_pamu_domain.h | 46 -
> drivers/iommu/intel/iommu.c | 55 --
> drivers/iommu/iommu.c | 75 ---
> drivers/soc/fsl/qbman/qman_portal.c | 56 --
> drivers/vfio/vfio_iommu_type1.c | 31 -
> drivers/vhost/vdpa.c | 10
> include/linux/iommu.h | 81 ---
> 16 files changed, 214 insertions(+), 1360 deletions(-)
Nice cleanup, thanks. The fsl_pamu driver and interface has always been
a little bit of an alien compared to other IOMMU drivers. I am inclined
to merge this after -rc3 is out, given some reviews. Can you also please
add changelogs to the last three patches?
Thanks,
Joerg
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-04 11:23 UTC (permalink / raw)
To: Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <CANpmjNOnVzei7frKcMzMHxaDXh5NvTA-Wpa29C2YC1GUxyKfhQ@mail.gmail.com>
Le 03/03/2021 à 11:56, Marco Elver a écrit :
>
> Somewhat tangentially, I also note that e.g. show_regs(regs) (which
> was printed along the KFENCE report above) didn't include the top
> frame in the "Call Trace", so this assumption is definitely not
> isolated to KFENCE.
>
Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
applied), and I get many failures. Any idea ?
[ 17.653751][ T58] ==================================================================
[ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
[ 17.654379][ T58]
[ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
[ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
[ 17.655775][ T58] .__slab_free+0x320/0x5a0
[ 17.656039][ T58] .test_double_free+0xe0/0x198
[ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
[ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.657161][ T58] .kthread+0x18c/0x1a0
[ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
[ 17.659869][ T58]
[ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
allocated by task 58:
[ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
[ 17.667069][ T58] .__kmalloc+0x280/0x4f0
[ 17.667452][ T58] .test_alloc+0x19c/0x430
[ 17.667732][ T58] .test_double_free+0x88/0x198
[ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
[ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.668553][ T58] .kthread+0x18c/0x1a0
[ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
[ 17.669711][ T58]
[ 17.669711][ T58] freed by task 58:
[ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
[ 17.670421][ T58] .__slab_free+0x320/0x5a0
[ 17.670603][ T58] .test_double_free+0xb4/0x198
[ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
[ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.671410][ T58] .kthread+0x18c/0x1a0
[ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
[ 17.671972][ T58]
[ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 17.673768][ T58] ==================================================================
[ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
[ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
[ 17.684397][ T1] not ok 7 - test_double_free
[ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
[ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
cache=1
[ 17.797584][ T59] ==================================================================
[ 17.801260][ T59] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
[ 17.801260][ T59]
[ 17.801512][ T59] Invalid free of 0xc00000003c9effe0 (in kfence-#78):
[ 17.801668][ T59] .kfence_guarded_free+0x2e4/0x530
[ 17.801849][ T59] .__slab_free+0x320/0x5a0
[ 17.801983][ T59] .kmem_cache_free+0x31c/0x5c0
[ 17.802109][ T59] .test_double_free+0xd0/0x198
[ 17.802227][ T59] .kunit_try_run_case+0x80/0x110
[ 17.802494][ T59] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.802641][ T59] .kthread+0x18c/0x1a0
[ 17.802821][ T59] .ret_from_kernel_thread+0x58/0x70
[ 17.802989][ T59]
[ 17.803303][ T59] kfence-#78 [0xc00000003c9effe0-0xc00000003c9effff, size=32, cache=test]
allocated by task 59:
[ 17.803666][ T59] .__kfence_alloc+0x1bc/0x510
[ 17.803898][ T59] .kmem_cache_alloc+0x290/0x440
[ 17.804036][ T59] .test_alloc+0x188/0x430
[ 17.804151][ T59] .test_double_free+0x88/0x198
[ 17.804363][ T59] .kunit_try_run_case+0x80/0x110
[ 17.804637][ T59] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.805099][ T59] .kthread+0x18c/0x1a0
[ 17.805313][ T59] .ret_from_kernel_thread+0x58/0x70
[ 17.806035][ T59]
[ 17.806035][ T59] freed by task 59:
[ 17.806495][ T59] .kfence_guarded_free+0x3d0/0x530
[ 17.806689][ T59] .__slab_free+0x320/0x5a0
[ 17.806941][ T59] .kmem_cache_free+0x31c/0x5c0
[ 17.807122][ T59] .test_double_free+0xa8/0x198
[ 17.807360][ T59] .kunit_try_run_case+0x80/0x110
[ 17.807538][ T59] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.807703][ T59] .kthread+0x18c/0x1a0
[ 17.808015][ T59] .ret_from_kernel_thread+0x58/0x70
[ 17.808220][ T59]
[ 17.808406][ T59] CPU: 0 PID: 59 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 17.808670][ T59] ==================================================================
[ 17.809882][ T59] # test_double_free-memcache: EXPECTATION FAILED at
mm/kfence/kfence_test.c:380
[ 17.809882][ T59] Expected report_matches(&expect) to be true, but is false
[ 17.812462][ T1] not ok 8 - test_double_free-memcache
[ 17.813872][ T60] # test_invalid_addr_free: test_alloc: size=32, gfp=cc0, policy=any, cache=0
[ 17.920432][ T60] ==================================================================
[ 17.920747][ T60] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
[ 17.920747][ T60]
[ 17.921003][ T60] Invalid free of 0xc00000003ca0ffe1 (in kfence-#79):
[ 17.921137][ T60] .kfence_guarded_free+0x2e4/0x530
[ 17.921268][ T60] .__slab_free+0x320/0x5a0
[ 17.921461][ T60] .test_invalid_addr_free+0xc0/0x1a8
[ 17.921607][ T60] .kunit_try_run_case+0x80/0x110
[ 17.921739][ T60] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.921946][ T60] .kthread+0x18c/0x1a0
[ 17.922065][ T60] .ret_from_kernel_thread+0x58/0x70
[ 17.922215][ T60]
[ 17.922327][ T60] kfence-#79 [0xc00000003ca0ffe0-0xc00000003ca0ffff, size=32, cache=kmalloc-32]
allocated by task 60:
[ 17.922585][ T60] .__kfence_alloc+0x1bc/0x510
[ 17.922730][ T60] .__kmalloc+0x280/0x4f0
[ 17.922903][ T60] .test_alloc+0x19c/0x430
[ 17.923032][ T60] .test_invalid_addr_free+0x8c/0x1a8
[ 17.923164][ T60] .kunit_try_run_case+0x80/0x110
[ 17.923333][ T60] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 17.923489][ T60] .kthread+0x18c/0x1a0
[ 17.923608][ T60] .ret_from_kernel_thread+0x58/0x70
[ 17.923773][ T60]
[ 17.923913][ T60] CPU: 0 PID: 60 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 17.924128][ T60] ==================================================================
[ 17.925133][ T60] # test_invalid_addr_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:397
[ 17.925133][ T60] Expected report_matches(&expect) to be true, but is false
[ 17.927243][ T1] not ok 9 - test_invalid_addr_free
[ 17.931185][ T61] # test_invalid_addr_free-memcache: setup_test_cache: size=32, ctor=0x0
[ 17.932183][ T61] # test_invalid_addr_free-memcache: test_alloc: size=32, gfp=cc0,
policy=any, cache=1
[ 18.149594][ T61] ==================================================================
[ 18.149909][ T61] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
[ 18.149909][ T61]
[ 18.150129][ T61] Invalid free of 0xc00000003ca20001 (in kfence-#80):
[ 18.150267][ T61] .kfence_guarded_free+0x2e4/0x530
[ 18.150465][ T61] .__slab_free+0x320/0x5a0
[ 18.150643][ T61] .kmem_cache_free+0x31c/0x5c0
[ 18.150873][ T61] .test_invalid_addr_free+0xb0/0x1a8
[ 18.151097][ T61] .kunit_try_run_case+0x80/0x110
[ 18.151286][ T61] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.151578][ T61] .kthread+0x18c/0x1a0
[ 18.151762][ T61] .ret_from_kernel_thread+0x58/0x70
[ 18.152085][ T61]
[ 18.152260][ T61] kfence-#80 [0xc00000003ca20000-0xc00000003ca2001f, size=32, cache=test]
allocated by task 61:
[ 18.152660][ T61] .__kfence_alloc+0x1bc/0x510
[ 18.152883][ T61] .kmem_cache_alloc+0x290/0x440
[ 18.153128][ T61] .test_alloc+0x188/0x430
[ 18.153325][ T61] .test_invalid_addr_free+0x8c/0x1a8
[ 18.153584][ T61] .kunit_try_run_case+0x80/0x110
[ 18.153796][ T61] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.154094][ T61] .kthread+0x18c/0x1a0
[ 18.154279][ T61] .ret_from_kernel_thread+0x58/0x70
[ 18.154555][ T61]
[ 18.154762][ T61] CPU: 0 PID: 61 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 18.155179][ T61] ==================================================================
[ 18.157795][ T61] # test_invalid_addr_free-memcache: EXPECTATION FAILED at
mm/kfence/kfence_test.c:397
[ 18.157795][ T61] Expected report_matches(&expect) to be true, but is false
[ 18.165055][ T1] not ok 10 - test_invalid_addr_free-memcache
[ 18.166459][ T62] # test_corruption: test_alloc: size=32, gfp=cc0, policy=left, cache=0
[ 18.514038][ T62] ==================================================================
[ 18.514772][ T62] BUG: KFENCE: memory corruption in .kfence_guarded_free+0x23c/0x530
[ 18.514772][ T62]
[ 18.515235][ T62] Corrupted memory at 0xc00000003ca60020 [ 0x2a . . . . . . . . . . . . . . . ]
(in kfence-#82):
[ 18.516308][ T62] .kfence_guarded_free+0x23c/0x530
[ 18.516550][ T62] .__slab_free+0x320/0x5a0
[ 18.516814][ T62] .test_corruption+0xcc/0x248
[ 18.517053][ T62] .kunit_try_run_case+0x80/0x110
[ 18.517347][ T62] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.517609][ T62] .kthread+0x18c/0x1a0
[ 18.517841][ T62] .ret_from_kernel_thread+0x58/0x70
[ 18.518271][ T62]
[ 18.518405][ T62] kfence-#82 [0xc00000003ca60000-0xc00000003ca6001f, size=32, cache=kmalloc-32]
allocated by task 62:
[ 18.518858][ T62] .__kfence_alloc+0x1bc/0x510
[ 18.519180][ T62] .__kmalloc+0x280/0x4f0
[ 18.519370][ T62] .test_alloc+0x19c/0x430
[ 18.519583][ T62] .test_corruption+0x94/0x248
[ 18.519864][ T62] .kunit_try_run_case+0x80/0x110
[ 18.520135][ T62] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.520368][ T62] .kthread+0x18c/0x1a0
[ 18.520569][ T62] .ret_from_kernel_thread+0x58/0x70
[ 18.520825][ T62]
[ 18.520990][ T62] CPU: 0 PID: 62 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 18.521803][ T62] ==================================================================
[ 18.523184][ T62] # test_corruption: EXPECTATION FAILED at mm/kfence/kfence_test.c:417
[ 18.523184][ T62] Expected report_matches(&expect) to be true, but is false
[ 18.524015][ T62] # test_corruption: test_alloc: size=32, gfp=cc0, policy=right, cache=0
[ 18.741141][ T62] ==================================================================
[ 18.741476][ T62] BUG: KFENCE: memory corruption in .kfence_guarded_free+0x160/0x530
[ 18.741476][ T62]
[ 18.741746][ T62] Corrupted memory at 0xc00000003ca8ffdf [ 0x2a ] (in kfence-#83):
[ 18.741995][ T62] .kfence_guarded_free+0x160/0x530
[ 18.742140][ T62] .__slab_free+0x320/0x5a0
[ 18.742402][ T62] .test_corruption+0x19c/0x248
[ 18.742586][ T62] .kunit_try_run_case+0x80/0x110
[ 18.742809][ T62] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.743027][ T62] .kthread+0x18c/0x1a0
[ 18.743488][ T62] .ret_from_kernel_thread+0x58/0x70
[ 18.743970][ T62]
[ 18.744156][ T62] kfence-#83 [0xc00000003ca8ffe0-0xc00000003ca8ffff, size=32, cache=kmalloc-32]
allocated by task 62:
[ 18.745071][ T62] .__kfence_alloc+0x1bc/0x510
[ 18.746079][ T62] .__kmalloc+0x280/0x4f0
[ 18.746595][ T62] .test_alloc+0x19c/0x430
[ 18.746879][ T62] .test_corruption+0x164/0x248
[ 18.748178][ T62] .kunit_try_run_case+0x80/0x110
[ 18.749636][ T62] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 18.749949][ T62] .kthread+0x18c/0x1a0
[ 18.750201][ T62] .ret_from_kernel_thread+0x58/0x70
[ 18.750369][ T62]
[ 18.750485][ T62] CPU: 0 PID: 62 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 18.750762][ T62] ==================================================================
[ 18.751578][ T62] # test_corruption: EXPECTATION FAILED at mm/kfence/kfence_test.c:423
[ 18.751578][ T62] Expected report_matches(&expect) to be true, but is false
[ 18.753340][ T1] not ok 11 - test_corruption
[ 18.754944][ T63] # test_corruption-memcache: setup_test_cache: size=32, ctor=0x0
[ 18.756995][ T63] # test_corruption-memcache: test_alloc: size=32, gfp=cc0, policy=left,
cache=1
[ 19.109763][ T63] ==================================================================
[ 19.110515][ T63] BUG: KFENCE: memory corruption in .kfence_guarded_free+0x23c/0x530
[ 19.110515][ T63]
[ 19.110978][ T63] Corrupted memory at 0xc00000003cac0020 [ 0x2a . . . . . . . . . . . . . . . ]
(in kfence-#85):
[ 19.111848][ T63] .kfence_guarded_free+0x23c/0x530
[ 19.112763][ T63] .__slab_free+0x320/0x5a0
[ 19.113121][ T63] .kmem_cache_free+0x31c/0x5c0
[ 19.113351][ T63] .test_corruption+0xc0/0x248
[ 19.113806][ T63] .kunit_try_run_case+0x80/0x110
[ 19.114019][ T63] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 19.114173][ T63] .kthread+0x18c/0x1a0
[ 19.114292][ T63] .ret_from_kernel_thread+0x58/0x70
[ 19.114535][ T63]
[ 19.114681][ T63] kfence-#85 [0xc00000003cac0000-0xc00000003cac001f, size=32, cache=test]
allocated by task 63:
[ 19.115287][ T63] .__kfence_alloc+0x1bc/0x510
[ 19.115597][ T63] .kmem_cache_alloc+0x290/0x440
[ 19.115875][ T63] .test_alloc+0x188/0x430
[ 19.116456][ T63] .test_corruption+0x94/0x248
[ 19.116689][ T63] .kunit_try_run_case+0x80/0x110
[ 19.117021][ T63] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 19.117383][ T63] .kthread+0x18c/0x1a0
[ 19.117572][ T63] .ret_from_kernel_thread+0x58/0x70
[ 19.117713][ T63]
[ 19.117816][ T63] CPU: 0 PID: 63 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 19.118706][ T63] ==================================================================
[ 19.119719][ T63] # test_corruption-memcache: EXPECTATION FAILED at mm/kfence/kfence_test.c:417
[ 19.119719][ T63] Expected report_matches(&expect) to be true, but is false
[ 19.120669][ T63] # test_corruption-memcache: test_alloc: size=32, gfp=cc0, policy=right,
cache=1
[ 19.339842][ T63] ==================================================================
[ 19.340238][ T63] BUG: KFENCE: memory corruption in .kfence_guarded_free+0x160/0x530
[ 19.340238][ T63]
[ 19.340487][ T63] Corrupted memory at 0xc00000003caeffdf [ 0x2a ] (in kfence-#86):
[ 19.340748][ T63] .kfence_guarded_free+0x160/0x530
[ 19.340885][ T63] .__slab_free+0x320/0x5a0
[ 19.341095][ T63] .kmem_cache_free+0x31c/0x5c0
[ 19.341285][ T63] .test_corruption+0x190/0x248
[ 19.341480][ T63] .kunit_try_run_case+0x80/0x110
[ 19.341921][ T63] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 19.342298][ T63] .kthread+0x18c/0x1a0
[ 19.342816][ T63] .ret_from_kernel_thread+0x58/0x70
[ 19.343799][ T63]
[ 19.345941][ T63] kfence-#86 [0xc00000003caeffe0-0xc00000003caeffff, size=32, cache=test]
allocated by task 63:
[ 19.347301][ T63] .__kfence_alloc+0x1bc/0x510
[ 19.347620][ T63] .kmem_cache_alloc+0x290/0x440
[ 19.347820][ T63] .test_alloc+0x188/0x430
[ 19.348202][ T63] .test_corruption+0x164/0x248
[ 19.348400][ T63] .kunit_try_run_case+0x80/0x110
[ 19.348609][ T63] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 19.348810][ T63] .kthread+0x18c/0x1a0
[ 19.349131][ T63] .ret_from_kernel_thread+0x58/0x70
[ 19.349429][ T63]
[ 19.349612][ T63] CPU: 0 PID: 63 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 19.349985][ T63] ==================================================================
[ 19.351145][ T63] # test_corruption-memcache: EXPECTATION FAILED at mm/kfence/kfence_test.c:423
[ 19.351145][ T63] Expected report_matches(&expect) to be true, but is false
[ 19.353730][ T1] not ok 12 - test_corruption-memcache
[ 26.350432][ T69] ==================================================================
[ 26.350757][ T69] BUG: KFENCE: memory corruption in .kfence_guarded_free+0x23c/0x530
[ 26.350757][ T69]
[ 26.350961][ T69] Corrupted memory at 0xc00000003cfcfff9 [ 0xac . . . . . . ] (in kfence-#125):
[ 26.351333][ T69] .kfence_guarded_free+0x23c/0x530
[ 26.351517][ T69] .__slab_free+0x320/0x5a0
[ 26.351645][ T69] .test_kmalloc_aligned_oob_write+0x12c/0x1e4
[ 26.351774][ T69] .kunit_try_run_case+0x80/0x110
[ 26.351902][ T69] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 26.352066][ T69] .kthread+0x18c/0x1a0
[ 26.352180][ T69] .ret_from_kernel_thread+0x58/0x70
[ 26.352294][ T69]
[ 26.352367][ T69] kfence-#125 [0xc00000003cfcffb0-0xc00000003cfcfff8, size=73,
cache=kmalloc-96] allocated by task 69:
[ 26.352668][ T69] .__kfence_alloc+0x1bc/0x510
[ 26.352804][ T69] .__kmalloc+0x280/0x4f0
[ 26.352913][ T69] .test_alloc+0x19c/0x430
[ 26.353092][ T69] .test_kmalloc_aligned_oob_write+0x80/0x1e4
[ 26.353290][ T69] .kunit_try_run_case+0x80/0x110
[ 26.353505][ T69] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 26.353724][ T69] .kthread+0x18c/0x1a0
[ 26.353896][ T69] .ret_from_kernel_thread+0x58/0x70
[ 26.354153][ T69]
[ 26.354309][ T69] CPU: 0 PID: 69 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 26.354714][ T69] ==================================================================
[ 26.355421][ T69] # test_kmalloc_aligned_oob_write: EXPECTATION FAILED at
mm/kfence/kfence_test.c:489
[ 26.355421][ T69] Expected report_matches(&expect) to be true, but is false
[ 26.357224][ T1] not ok 18 - test_kmalloc_aligned_oob_write
Christophe
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Marco Elver @ 2021-03-04 11:31 UTC (permalink / raw)
To: Christophe Leroy
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <f036c53d-7e81-763c-47f4-6024c6c5f058@csgroup.eu>
On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 03/03/2021 à 11:56, Marco Elver a écrit :
> >
> > Somewhat tangentially, I also note that e.g. show_regs(regs) (which
> > was printed along the KFENCE report above) didn't include the top
> > frame in the "Call Trace", so this assumption is definitely not
> > isolated to KFENCE.
> >
>
> Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
> applied), and I get many failures. Any idea ?
>
> [ 17.653751][ T58] ==================================================================
> [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
> [ 17.654379][ T58]
> [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
> [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
> [ 17.655775][ T58] .__slab_free+0x320/0x5a0
> [ 17.656039][ T58] .test_double_free+0xe0/0x198
> [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
> [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 17.657161][ T58] .kthread+0x18c/0x1a0
> [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
> [ 17.659869][ T58]
> [ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
> allocated by task 58:
> [ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
> [ 17.667069][ T58] .__kmalloc+0x280/0x4f0
> [ 17.667452][ T58] .test_alloc+0x19c/0x430
> [ 17.667732][ T58] .test_double_free+0x88/0x198
> [ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
> [ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 17.668553][ T58] .kthread+0x18c/0x1a0
> [ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
> [ 17.669711][ T58]
> [ 17.669711][ T58] freed by task 58:
> [ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
> [ 17.670421][ T58] .__slab_free+0x320/0x5a0
> [ 17.670603][ T58] .test_double_free+0xb4/0x198
> [ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
> [ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 17.671410][ T58] .kthread+0x18c/0x1a0
> [ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
> [ 17.671972][ T58]
> [ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
> [ 17.673768][ T58] ==================================================================
> [ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
> [ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
> [ 17.684397][ T1] not ok 7 - test_double_free
> [ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
> [ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
> cache=1
Looks like something is prepending '.' to function names. We expect
the function name to appear as-is, e.g. "kfence_guarded_free",
"test_double_free", etc.
Is there something special on ppc64, where the '.' is some convention?
Thanks,
-- Marco
^ permalink raw reply
* Re: [PATCH v2] powerpc: Fix save_stack_trace_regs() to have running function as first entry
From: Michael Ellerman @ 2021-03-04 11:36 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, elver,
rostedt
Cc: linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <20dad21f9446938697573e6642db583bdb874656.1614792440.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> It seems like other architectures, namely x86 and arm64
> at least, include the running function as top entry when saving
> stack trace with save_stack_trace_regs().
Also riscv AFAICS.
> Functionnalities like KFENCE expect it.
>
> Do the same on powerpc, it allows KFENCE to properly identify the faulting
> function as depicted below. Before the patch KFENCE was identifying
> finish_task_switch.isra as the faulting function.
Thanks, I think this is the right approach. There's kfence but also
several other users from what I can see with a quick grep.
...
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Fixes: 35de3b1aa168 ("powerpc: Implement save_stack_trace_regs() to enable kprobe stack tracing")
> Cc: stable@vger.kernel.org
I'm not sure about the Cc to stable. I think we are fixing the behaviour
to match the (implied) intent of the API, but that doesn't mean we won't
break something by accident. I'll think about it :)
cheers
^ permalink raw reply
* [PATCH] powerpc/pseries: export LPAR security flavor in lparcfg
From: Laurent Dufour @ 2021-03-04 11:42 UTC (permalink / raw)
To: mpe, benh, paulus, linuxppc-dev; +Cc: nathanl, cheloha, linux-kernel
This is helpful to read the security flavor from inside the LPAR.
Export it like this in /proc/powerpc/lparcfg:
$ grep security_flavor /proc/powerpc/lparcfg
security_flavor=1
Value means:
0 Speculative execution fully enabled
1 Speculative execution controls to mitigate user-to-kernel attacks
2 Speculative execution controls to mitigate user-to-kernel and
user-to-user side-channel attacks
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/platforms/pseries/lparcfg.c | 2 ++
arch/powerpc/platforms/pseries/pseries.h | 1 +
arch/powerpc/platforms/pseries/setup.c | 8 ++++++++
4 files changed, 12 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index ed6086d57b22..455e188da26d 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -389,6 +389,7 @@
#define H_CPU_BEHAV_FAVOUR_SECURITY (1ull << 63) // IBM bit 0
#define H_CPU_BEHAV_L1D_FLUSH_PR (1ull << 62) // IBM bit 1
#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ull << 61) // IBM bit 2
+#define H_CPU_BEHAV_FAVOUR_SECURITY_H (1ull << 60) // IBM bit 3
#define H_CPU_BEHAV_FLUSH_COUNT_CACHE (1ull << 58) // IBM bit 5
#define H_CPU_BEHAV_FLUSH_LINK_STACK (1ull << 57) // IBM bit 6
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index e278390ab28d..35f6c4929fbd 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -169,6 +169,7 @@ static void show_gpci_data(struct seq_file *m)
kfree(buf);
}
+
static unsigned h_pic(unsigned long *pool_idle_time,
unsigned long *num_procs)
{
@@ -537,6 +538,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v)
parse_em_data(m);
maxmem_data(m);
+ seq_printf(m, "security_flavor=%u\n", pseries_security_flavor);
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 4fe48c04c6c2..a25517dc2515 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -111,6 +111,7 @@ static inline unsigned long cmo_get_page_size(void)
int dlpar_workqueue_init(void);
+extern u32 pseries_security_flavor;
void pseries_setup_security_mitigations(void);
void pseries_lpar_read_hblkrm_characteristics(void);
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 46e1540abc22..59080413a269 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -85,6 +85,7 @@ EXPORT_SYMBOL(CMO_PageSize);
int fwnmi_active; /* TRUE if an FWNMI handler is present */
int ibm_nmi_interlock_token;
+u32 pseries_security_flavor;
static void pSeries_show_cpuinfo(struct seq_file *m)
{
@@ -534,9 +535,16 @@ static void init_cpu_char_feature_flags(struct h_cpu_char_result *result)
/*
* The features below are enabled by default, so we instead look to see
* if firmware has *disabled* them, and clear them if so.
+ * H_CPU_BEHAV_FAVOUR_SECURITY_H could be set only if
+ * H_CPU_BEHAV_FAVOUR_SECURITY is.
*/
if (!(result->behaviour & H_CPU_BEHAV_FAVOUR_SECURITY))
security_ftr_clear(SEC_FTR_FAVOUR_SECURITY);
+ else if (result->behaviour & H_CPU_BEHAV_FAVOUR_SECURITY_H)
+ pseries_security_flavor = 1;
+ else
+ pseries_security_flavor = 2;
+
if (!(result->behaviour & H_CPU_BEHAV_L1D_FLUSH_PR))
security_ftr_clear(SEC_FTR_L1D_FLUSH_PR);
--
2.30.1
^ permalink raw reply related
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2021-03-04 11:44 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
--- Comment #7 from Michael Ellerman (michael@ellerman.id.au) ---
Yeah, reverting that on mainline fixed it.
Not sure how we're going to fix it though, a straight revert isn't going to be
acceptable I suspect.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-04 11:48 UTC (permalink / raw)
To: Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <CANpmjNMn_CUrgeSqBgiKx4+J8a+XcxkaLPWoDMUvUEXk8+-jxg@mail.gmail.com>
Le 04/03/2021 à 12:31, Marco Elver a écrit :
> On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 03/03/2021 à 11:56, Marco Elver a écrit :
>>>
>>> Somewhat tangentially, I also note that e.g. show_regs(regs) (which
>>> was printed along the KFENCE report above) didn't include the top
>>> frame in the "Call Trace", so this assumption is definitely not
>>> isolated to KFENCE.
>>>
>>
>> Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
>> applied), and I get many failures. Any idea ?
>>
>> [ 17.653751][ T58] ==================================================================
>> [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
>> [ 17.654379][ T58]
>> [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
>> [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
>> [ 17.655775][ T58] .__slab_free+0x320/0x5a0
>> [ 17.656039][ T58] .test_double_free+0xe0/0x198
>> [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
>> [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>> [ 17.657161][ T58] .kthread+0x18c/0x1a0
>> [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
>> [ 17.659869][ T58]
>> [ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
>> allocated by task 58:
>> [ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
>> [ 17.667069][ T58] .__kmalloc+0x280/0x4f0
>> [ 17.667452][ T58] .test_alloc+0x19c/0x430
>> [ 17.667732][ T58] .test_double_free+0x88/0x198
>> [ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
>> [ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>> [ 17.668553][ T58] .kthread+0x18c/0x1a0
>> [ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
>> [ 17.669711][ T58]
>> [ 17.669711][ T58] freed by task 58:
>> [ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
>> [ 17.670421][ T58] .__slab_free+0x320/0x5a0
>> [ 17.670603][ T58] .test_double_free+0xb4/0x198
>> [ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
>> [ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>> [ 17.671410][ T58] .kthread+0x18c/0x1a0
>> [ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
>> [ 17.671972][ T58]
>> [ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
>> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
>> [ 17.673768][ T58] ==================================================================
>> [ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
>> [ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
>> [ 17.684397][ T1] not ok 7 - test_double_free
>> [ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
>> [ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
>> cache=1
>
> Looks like something is prepending '.' to function names. We expect
> the function name to appear as-is, e.g. "kfence_guarded_free",
> "test_double_free", etc.
>
> Is there something special on ppc64, where the '.' is some convention?
>
I think so, see https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
Also see commit https://github.com/linuxppc/linux/commit/02424d896
Christophe
^ permalink raw reply
* [PATCH] powerpc/perf: Fix sampled instruction type for larx/stcx
From: Athira Rajeev @ 2021-03-04 11:55 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev
Sampled Instruction Event Register (SIER) field [46:48]
identifies the sampled instruction type. ISA v3.1 says value
of 0b111 for this field as reserved, but in POWER10 it denotes
LARX/STCX type which will hopefully be fixed in ISA v3.1 update.
Patch fixes the functions to handle type value 7 for
CPU_FTR_ARCH_31.
Fixes: a64e697cef23 ("powerpc/perf: power10 Performance Monitoring support")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/perf/isa207-common.c | 30 +++++++++++++++++++++++++++---
arch/powerpc/perf/isa207-common.h | 1 +
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index e4f577da33d8..754f904d8d69 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -266,6 +266,8 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
u32 sub_idx;
u64 sier;
u64 val;
+ u64 mmcra = mfspr(SPRN_MMCRA);
+ u32 op_type;
/* Skip if no SIER support */
if (!(flags & PPMU_HAS_SIER)) {
@@ -275,12 +277,34 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
sier = mfspr(SPRN_SIER);
val = (sier & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
- if (val == 1 || val == 2) {
+ if (val == 1 || val == 2 || (val == 7 && cpu_has_feature(CPU_FTR_ARCH_31))) {
idx = (sier & ISA207_SIER_LDST_MASK) >> ISA207_SIER_LDST_SHIFT;
sub_idx = (sier & ISA207_SIER_DATA_SRC_MASK) >> ISA207_SIER_DATA_SRC_SHIFT;
dsrc->val = isa207_find_source(idx, sub_idx);
- dsrc->val |= (val == 1) ? P(OP, LOAD) : P(OP, STORE);
+ if (val == 7) {
+ /*
+ * Type 0b111 denotes either larx or stcx instruction. Use the
+ * MMCRA sampling bits [57:59] along with the type value
+ * to determine the exact instruction type. If the sampling
+ * criteria is neither load or store, set the type as default
+ * to NA.
+ */
+ op_type = (mmcra >> MMCRA_SAMP_ELIG_SHIFT) & MMCRA_SAMP_ELIG_MASK;
+ switch (op_type) {
+ case 5:
+ dsrc->val |= P(OP, LOAD);
+ break;
+ case 7:
+ dsrc->val |= P(OP, STORE);
+ break;
+ default:
+ dsrc->val |= P(OP, NA);
+ break;
+ }
+ } else {
+ dsrc->val |= (val == 1) ? P(OP, LOAD) : P(OP, STORE);
+ }
}
}
@@ -295,7 +319,7 @@ void isa207_get_mem_weight(u64 *weight)
if (cpu_has_feature(CPU_FTR_ARCH_31))
mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
- if (val == 0 || val == 7)
+ if (val == 0 || (val == 7 && !cpu_has_feature(CPU_FTR_ARCH_31)))
*weight = 0;
else
*weight = mantissa << (2 * exp);
diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
index 1af0e8c97ac7..7b0242efe4b9 100644
--- a/arch/powerpc/perf/isa207-common.h
+++ b/arch/powerpc/perf/isa207-common.h
@@ -220,6 +220,7 @@
/* Bits in MMCRA for PowerISA v2.07 */
#define MMCRA_SAMP_MODE_SHIFT 1
#define MMCRA_SAMP_ELIG_SHIFT 4
+#define MMCRA_SAMP_ELIG_MASK 7
#define MMCRA_THR_CTL_SHIFT 8
#define MMCRA_THR_SEL_SHIFT 16
#define MMCRA_THR_CMP_SHIFT 32
--
1.8.3.1
^ permalink raw reply related
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2021-03-04 11:56 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
--- Comment #8 from Michael Ellerman (michael@ellerman.id.au) ---
Actually I also reverted 4e302c3b568e ("misc: eeprom: at24: fix NVMEM name with
custom AT24 device name").
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-04 12:00 UTC (permalink / raw)
To: Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <7270e1cc-bb6b-99ee-0043-08a027b8d83a@csgroup.eu>
Le 04/03/2021 à 12:48, Christophe Leroy a écrit :
>
>
> Le 04/03/2021 à 12:31, Marco Elver a écrit :
>> On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
>> <christophe.leroy@csgroup.eu> wrote:
>>> Le 03/03/2021 à 11:56, Marco Elver a écrit :
>>>>
>>>> Somewhat tangentially, I also note that e.g. show_regs(regs) (which
>>>> was printed along the KFENCE report above) didn't include the top
>>>> frame in the "Call Trace", so this assumption is definitely not
>>>> isolated to KFENCE.
>>>>
>>>
>>> Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
>>> applied), and I get many failures. Any idea ?
>>>
>>> [ 17.653751][ T58] ==================================================================
>>> [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
>>> [ 17.654379][ T58]
>>> [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
>>> [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
>>> [ 17.655775][ T58] .__slab_free+0x320/0x5a0
>>> [ 17.656039][ T58] .test_double_free+0xe0/0x198
>>> [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
>>> [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>>> [ 17.657161][ T58] .kthread+0x18c/0x1a0
>>> [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
>>> [ 17.659869][ T58]
>>> [ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
>>> allocated by task 58:
>>> [ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
>>> [ 17.667069][ T58] .__kmalloc+0x280/0x4f0
>>> [ 17.667452][ T58] .test_alloc+0x19c/0x430
>>> [ 17.667732][ T58] .test_double_free+0x88/0x198
>>> [ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
>>> [ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>>> [ 17.668553][ T58] .kthread+0x18c/0x1a0
>>> [ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
>>> [ 17.669711][ T58]
>>> [ 17.669711][ T58] freed by task 58:
>>> [ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
>>> [ 17.670421][ T58] .__slab_free+0x320/0x5a0
>>> [ 17.670603][ T58] .test_double_free+0xb4/0x198
>>> [ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
>>> [ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
>>> [ 17.671410][ T58] .kthread+0x18c/0x1a0
>>> [ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
>>> [ 17.671972][ T58]
>>> [ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
>>> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
>>> [ 17.673768][ T58] ==================================================================
>>> [ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
>>> [ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
>>> [ 17.684397][ T1] not ok 7 - test_double_free
>>> [ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
>>> [ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
>>> cache=1
>>
>> Looks like something is prepending '.' to function names. We expect
>> the function name to appear as-is, e.g. "kfence_guarded_free",
>> "test_double_free", etc.
>>
>> Is there something special on ppc64, where the '.' is some convention?
>>
>
> I think so, see https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
>
> Also see commit https://github.com/linuxppc/linux/commit/02424d896
>
But I'm wondering, if the dot is the problem, how so is the following one ok ?
[ 79.574457][ T75] # test_krealloc: test_alloc: size=32, gfp=cc0, policy=any, cache=0
[ 79.682728][ T75] ==================================================================
[ 79.684017][ T75] BUG: KFENCE: use-after-free read in .test_krealloc+0x4fc/0x5b8
[ 79.684017][ T75]
[ 79.684955][ T75] Use-after-free read at 0xc00000003d060000 (in kfence-#130):
[ 79.687581][ T75] .test_krealloc+0x4fc/0x5b8
[ 79.688216][ T75] .test_krealloc+0x4e4/0x5b8
[ 79.688824][ T75] .kunit_try_run_case+0x80/0x110
[ 79.689737][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 79.690335][ T75] .kthread+0x18c/0x1a0
[ 79.691092][ T75] .ret_from_kernel_thread+0x58/0x70
[ 79.692081][ T75]
[ 79.692671][ T75] kfence-#130 [0xc00000003d060000-0xc00000003d06001f, size=32,
cache=kmalloc-32] allocated by task 75:
[ 79.700977][ T75] .__kfence_alloc+0x1bc/0x510
[ 79.701812][ T75] .__kmalloc+0x280/0x4f0
[ 79.702695][ T75] .test_alloc+0x19c/0x430
[ 79.703051][ T75] .test_krealloc+0xa8/0x5b8
[ 79.703276][ T75] .kunit_try_run_case+0x80/0x110
[ 79.703693][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 79.704223][ T75] .kthread+0x18c/0x1a0
[ 79.704586][ T75] .ret_from_kernel_thread+0x58/0x70
[ 79.704968][ T75]
[ 79.704968][ T75] freed by task 75:
[ 79.705756][ T75] .kfence_guarded_free+0x3d0/0x530
[ 79.706754][ T75] .__slab_free+0x320/0x5a0
[ 79.708575][ T75] .krealloc+0xe8/0x180
[ 79.708970][ T75] .test_krealloc+0x1c8/0x5b8
[ 79.709606][ T75] .kunit_try_run_case+0x80/0x110
[ 79.710204][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
[ 79.710639][ T75] .kthread+0x18c/0x1a0
[ 79.710996][ T75] .ret_from_kernel_thread+0x58/0x70
[ 79.711349][ T75]
[ 79.717435][ T75] CPU: 0 PID: 75 Comm: kunit_try_catch Tainted: G B
5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
[ 79.718124][ T75] NIP: c000000000468a40 LR: c000000000468a28 CTR: 0000000000000000
[ 79.727741][ T75] REGS: c000000007dd3830 TRAP: 0300 Tainted: G B
(5.12.0-rc1-01540-g0783285cc1b8-dirty)
[ 79.733377][ T75] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 28000440 XER: 00000000
[ 79.738770][ T75] CFAR: c000000000888c7c DAR: c00000003d060000 DSISR: 40000000 IRQMASK: 0
[ 79.738770][ T75] GPR00: c000000000468a28 c000000007dd3ad0 c000000001eaad00 c0000000073c3988
[ 79.738770][ T75] GPR04: c000000007dd3b60 0000000000000001 0000000000000000 c00000003d060000
[ 79.738770][ T75] GPR08: 00000000000002c8 0000000000000001 c0000000011bb410 c00000003fe903d8
[ 79.738770][ T75] GPR12: 0000000028000440 c0000000020f0000 c0000000001a6460 c00000000724bb80
[ 79.738770][ T75] GPR16: 0000000000000000 c00000000731749f c0000000011bb278 c00000000731749f
[ 79.738770][ T75] GPR20: 00000001000002c1 0000000000000000 c0000000011bb278 c0000000011bb3b8
[ 79.738770][ T75] GPR24: c0000000073174a0 c0000000011aa7b8 c000000001e35328 c00000000208ad00
[ 79.738770][ T75] GPR28: 0000000000000000 c0000000011bb0b8 c0000000073c3988 c000000007dd3ad0
[ 79.751744][ T75] NIP [c000000000468a40] .test_krealloc+0x4fc/0x5b8
[ 79.752243][ T75] LR [c000000000468a28] .test_krealloc+0x4e4/0x5b8
[ 79.752699][ T75] Call Trace:
[ 79.753027][ T75] [c000000007dd3ad0] [c000000000468a28] .test_krealloc+0x4e4/0x5b8 (unreliable)
[ 79.753878][ T75] [c000000007dd3c40] [c0000000008886d0] .kunit_try_run_case+0x80/0x110
[ 79.754641][ T75] [c000000007dd3cd0] [c00000000088a808]
.kunit_generic_run_threadfn_adapter+0x38/0x50
[ 79.755494][ T75] [c000000007dd3d50] [c0000000001a65ec] .kthread+0x18c/0x1a0
[ 79.757254][ T75] [c000000007dd3e10] [c00000000000dd68] .ret_from_kernel_thread+0x58/0x70
[ 79.775521][ T75] Instruction dump:
[ 79.776890][ T75] 68a50001 9b9f00c8 fbdf0090 fbbf00a0 fb5f00b8 484201cd 60000000 e8ff0080
[ 79.783146][ T75] 3d42ff31 390002c8 394a0710 39200001 <88e70000> 38a00000 fb9f00a8 e8fbe80e
[ 79.787563][ T75] ==================================================================
[ 79.804667][ T1] ok 24 - test_krealloc
Christophe
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Marco Elver @ 2021-03-04 12:02 UTC (permalink / raw)
To: Christophe Leroy
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <72e31c34-e947-1084-2bd2-f5b80786f827@csgroup.eu>
On Thu, 4 Mar 2021 at 13:00, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 04/03/2021 à 12:48, Christophe Leroy a écrit :
> >
> >
> > Le 04/03/2021 à 12:31, Marco Elver a écrit :
> >> On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
> >> <christophe.leroy@csgroup.eu> wrote:
> >>> Le 03/03/2021 à 11:56, Marco Elver a écrit :
> >>>>
> >>>> Somewhat tangentially, I also note that e.g. show_regs(regs) (which
> >>>> was printed along the KFENCE report above) didn't include the top
> >>>> frame in the "Call Trace", so this assumption is definitely not
> >>>> isolated to KFENCE.
> >>>>
> >>>
> >>> Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
> >>> applied), and I get many failures. Any idea ?
> >>>
> >>> [ 17.653751][ T58] ==================================================================
> >>> [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
> >>> [ 17.654379][ T58]
> >>> [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
> >>> [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
> >>> [ 17.655775][ T58] .__slab_free+0x320/0x5a0
> >>> [ 17.656039][ T58] .test_double_free+0xe0/0x198
> >>> [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.657161][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.659869][ T58]
> >>> [ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
> >>> allocated by task 58:
> >>> [ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
> >>> [ 17.667069][ T58] .__kmalloc+0x280/0x4f0
> >>> [ 17.667452][ T58] .test_alloc+0x19c/0x430
> >>> [ 17.667732][ T58] .test_double_free+0x88/0x198
> >>> [ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.668553][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.669711][ T58]
> >>> [ 17.669711][ T58] freed by task 58:
> >>> [ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
> >>> [ 17.670421][ T58] .__slab_free+0x320/0x5a0
> >>> [ 17.670603][ T58] .test_double_free+0xb4/0x198
> >>> [ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.671410][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.671972][ T58]
> >>> [ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
> >>> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
> >>> [ 17.673768][ T58] ==================================================================
> >>> [ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
> >>> [ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
> >>> [ 17.684397][ T1] not ok 7 - test_double_free
> >>> [ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
> >>> [ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
> >>> cache=1
> >>
> >> Looks like something is prepending '.' to function names. We expect
> >> the function name to appear as-is, e.g. "kfence_guarded_free",
> >> "test_double_free", etc.
> >>
> >> Is there something special on ppc64, where the '.' is some convention?
> >>
> >
> > I think so, see https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
> >
> > Also see commit https://github.com/linuxppc/linux/commit/02424d896
> >
>
> But I'm wondering, if the dot is the problem, how so is the following one ok ?
>
> [ 79.574457][ T75] # test_krealloc: test_alloc: size=32, gfp=cc0, policy=any, cache=0
> [ 79.682728][ T75] ==================================================================
> [ 79.684017][ T75] BUG: KFENCE: use-after-free read in .test_krealloc+0x4fc/0x5b8
> [ 79.684017][ T75]
> [ 79.684955][ T75] Use-after-free read at 0xc00000003d060000 (in kfence-#130):
> [ 79.687581][ T75] .test_krealloc+0x4fc/0x5b8
> [ 79.688216][ T75] .test_krealloc+0x4e4/0x5b8
> [ 79.688824][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.689737][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.690335][ T75] .kthread+0x18c/0x1a0
> [ 79.691092][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.692081][ T75]
> [ 79.692671][ T75] kfence-#130 [0xc00000003d060000-0xc00000003d06001f, size=32,
> cache=kmalloc-32] allocated by task 75:
> [ 79.700977][ T75] .__kfence_alloc+0x1bc/0x510
> [ 79.701812][ T75] .__kmalloc+0x280/0x4f0
> [ 79.702695][ T75] .test_alloc+0x19c/0x430
> [ 79.703051][ T75] .test_krealloc+0xa8/0x5b8
> [ 79.703276][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.703693][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.704223][ T75] .kthread+0x18c/0x1a0
> [ 79.704586][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.704968][ T75]
> [ 79.704968][ T75] freed by task 75:
> [ 79.705756][ T75] .kfence_guarded_free+0x3d0/0x530
> [ 79.706754][ T75] .__slab_free+0x320/0x5a0
> [ 79.708575][ T75] .krealloc+0xe8/0x180
> [ 79.708970][ T75] .test_krealloc+0x1c8/0x5b8
> [ 79.709606][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.710204][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.710639][ T75] .kthread+0x18c/0x1a0
> [ 79.710996][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.711349][ T75]
> [ 79.717435][ T75] CPU: 0 PID: 75 Comm: kunit_try_catch Tainted: G B
> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
> [ 79.718124][ T75] NIP: c000000000468a40 LR: c000000000468a28 CTR: 0000000000000000
> [ 79.727741][ T75] REGS: c000000007dd3830 TRAP: 0300 Tainted: G B
> (5.12.0-rc1-01540-g0783285cc1b8-dirty)
> [ 79.733377][ T75] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 28000440 XER: 00000000
> [ 79.738770][ T75] CFAR: c000000000888c7c DAR: c00000003d060000 DSISR: 40000000 IRQMASK: 0
> [ 79.738770][ T75] GPR00: c000000000468a28 c000000007dd3ad0 c000000001eaad00 c0000000073c3988
> [ 79.738770][ T75] GPR04: c000000007dd3b60 0000000000000001 0000000000000000 c00000003d060000
> [ 79.738770][ T75] GPR08: 00000000000002c8 0000000000000001 c0000000011bb410 c00000003fe903d8
> [ 79.738770][ T75] GPR12: 0000000028000440 c0000000020f0000 c0000000001a6460 c00000000724bb80
> [ 79.738770][ T75] GPR16: 0000000000000000 c00000000731749f c0000000011bb278 c00000000731749f
> [ 79.738770][ T75] GPR20: 00000001000002c1 0000000000000000 c0000000011bb278 c0000000011bb3b8
> [ 79.738770][ T75] GPR24: c0000000073174a0 c0000000011aa7b8 c000000001e35328 c00000000208ad00
> [ 79.738770][ T75] GPR28: 0000000000000000 c0000000011bb0b8 c0000000073c3988 c000000007dd3ad0
> [ 79.751744][ T75] NIP [c000000000468a40] .test_krealloc+0x4fc/0x5b8
> [ 79.752243][ T75] LR [c000000000468a28] .test_krealloc+0x4e4/0x5b8
> [ 79.752699][ T75] Call Trace:
> [ 79.753027][ T75] [c000000007dd3ad0] [c000000000468a28] .test_krealloc+0x4e4/0x5b8 (unreliable)
> [ 79.753878][ T75] [c000000007dd3c40] [c0000000008886d0] .kunit_try_run_case+0x80/0x110
> [ 79.754641][ T75] [c000000007dd3cd0] [c00000000088a808]
> .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.755494][ T75] [c000000007dd3d50] [c0000000001a65ec] .kthread+0x18c/0x1a0
> [ 79.757254][ T75] [c000000007dd3e10] [c00000000000dd68] .ret_from_kernel_thread+0x58/0x70
> [ 79.775521][ T75] Instruction dump:
> [ 79.776890][ T75] 68a50001 9b9f00c8 fbdf0090 fbbf00a0 fb5f00b8 484201cd 60000000 e8ff0080
> [ 79.783146][ T75] 3d42ff31 390002c8 394a0710 39200001 <88e70000> 38a00000 fb9f00a8 e8fbe80e
> [ 79.787563][ T75] ==================================================================
> [ 79.804667][ T1] ok 24 - test_krealloc
This one is using pt_regs, and therefore isn't trying to determine how
many entries we can skip in the stack trace to avoid showing
internals. I'll reply with a potential solution you can test shortly.
Thanks,
-- Marco
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Marco Elver @ 2021-03-04 12:48 UTC (permalink / raw)
To: Christophe Leroy
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <7270e1cc-bb6b-99ee-0043-08a027b8d83a@csgroup.eu>
On Thu, Mar 04, 2021 at 12:48PM +0100, Christophe Leroy wrote:
> Le 04/03/2021 à 12:31, Marco Elver a écrit :
> > On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> > > Le 03/03/2021 à 11:56, Marco Elver a écrit :
> > > >
> > > > Somewhat tangentially, I also note that e.g. show_regs(regs) (which
> > > > was printed along the KFENCE report above) didn't include the top
> > > > frame in the "Call Trace", so this assumption is definitely not
> > > > isolated to KFENCE.
> > > >
> > >
> > > Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
> > > applied), and I get many failures. Any idea ?
> > >
> > > [ 17.653751][ T58] ==================================================================
> > > [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
> > > [ 17.654379][ T58]
> > > [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
> > > [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
> > > [ 17.655775][ T58] .__slab_free+0x320/0x5a0
> > > [ 17.656039][ T58] .test_double_free+0xe0/0x198
> > > [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
> > > [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> > > [ 17.657161][ T58] .kthread+0x18c/0x1a0
> > > [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
> > > [ 17.659869][ T58]
[...]
> >
> > Looks like something is prepending '.' to function names. We expect
> > the function name to appear as-is, e.g. "kfence_guarded_free",
> > "test_double_free", etc.
> >
> > Is there something special on ppc64, where the '.' is some convention?
> >
>
> I think so, see https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
>
> Also see commit https://github.com/linuxppc/linux/commit/02424d896
Thanks -- could you try the below patch? You'll need to define
ARCH_FUNC_PREFIX accordingly.
We think, since there are only very few architectures that add a prefix,
requiring <asm/kfence.h> to define something like ARCH_FUNC_PREFIX is
the simplest option. Let me know if this works for you.
There an alternative option, which is to dynamically figure out the
prefix, but if this simpler option is fine with you, we'd prefer it.
Thanks,
-- Marco
------ >8 ------
From d118080eb9552073f5dcf1f86198f3d86d5ea850 Mon Sep 17 00:00:00 2001
From: Marco Elver <elver@google.com>
Date: Thu, 4 Mar 2021 13:15:51 +0100
Subject: [PATCH] kfence: fix reports if constant function prefixes exist
Some architectures prefix all functions with a constant string ('.' on
ppc64). Add ARCH_FUNC_PREFIX, which may optionally be defined in
<asm/kfence.h>, so that get_stack_skipnr() can work properly.
Link: https://lkml.kernel.org/r/f036c53d-7e81-763c-47f4-6024c6c5f058@csgroup.eu
Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Marco Elver <elver@google.com>
---
mm/kfence/report.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/mm/kfence/report.c b/mm/kfence/report.c
index 519f037720f5..e3f71451ad9e 100644
--- a/mm/kfence/report.c
+++ b/mm/kfence/report.c
@@ -20,6 +20,11 @@
#include "kfence.h"
+/* May be overridden by <asm/kfence.h>. */
+#ifndef ARCH_FUNC_PREFIX
+#define ARCH_FUNC_PREFIX ""
+#endif
+
extern bool no_hash_pointers;
/* Helper function to either print to a seq_file or to console. */
@@ -67,8 +72,9 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
for (skipnr = 0; skipnr < num_entries; skipnr++) {
int len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skipnr]);
- if (str_has_prefix(buf, "kfence_") || str_has_prefix(buf, "__kfence_") ||
- !strncmp(buf, "__slab_free", len)) {
+ if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfence_") ||
+ str_has_prefix(buf, ARCH_FUNC_PREFIX "__kfence_") ||
+ !strncmp(buf, ARCH_FUNC_PREFIX "__slab_free", len)) {
/*
* In case of tail calls from any of the below
* to any of the above.
@@ -77,10 +83,10 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
}
/* Also the *_bulk() variants by only checking prefixes. */
- if (str_has_prefix(buf, "kfree") ||
- str_has_prefix(buf, "kmem_cache_free") ||
- str_has_prefix(buf, "__kmalloc") ||
- str_has_prefix(buf, "kmem_cache_alloc"))
+ if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfree") ||
+ str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_free") ||
+ str_has_prefix(buf, ARCH_FUNC_PREFIX "__kmalloc") ||
+ str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_alloc"))
goto found;
}
if (fallback < num_entries)
--
2.30.1.766.gb4fecdf3b7-goog
^ permalink raw reply related
* [PATCH v2] powerpc/32: remove bogus ppc_select syscall
From: Christophe Leroy @ 2021-03-04 12:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, arnd
Cc: halesh.sadashiv, linuxppc-dev, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The ppc_select function was introduced in linux-2.3.48 in order to support
code confusing the legacy select() calling convention with the standard one.
Even 24 years ago, all correctly built code should not have done this and
could have easily been phased out. Nothing that was compiled later should
actually try to use the old_select interface, and it would have been broken
already on all ppc64 kernels with the syscall emulation layer.
This patch brings the 32 bit compat ABI and the native 32 bit ABI for
powerpc into a consistent state, by removing support for both the
old_select system call number and the handler for it.
The bug report triggering this came from
Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>, who discovered that the
32 bit implementation of ppc_select would in case of a negative number
of file descriptors incorrectly return -EFAULT instead of -EINVAL.
There seems to be no way to fix this problem in a way that would
keep broken pre-1997 binaries running.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>
[chleroy: Rebased and updated the number of years elapsed in the commit message]
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
First version was in 2008, at that time it was rejected, see
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/
If we decide to still keep this, then we'll have to:
- take into account -4096 < fd < 0 case
- use unsafe_get_user inside a uaccess_begin block
---
arch/powerpc/include/asm/asm-prototypes.h | 3 ---
arch/powerpc/kernel/syscalls.c | 25 -----------------------
arch/powerpc/kernel/syscalls/syscall.tbl | 4 +---
3 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 939f3c94c8f3..78e0a3bd448a 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -63,9 +63,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
#ifdef CONFIG_PPC32
long sys_debug_setcontext(struct ucontext __user *ctx,
int ndbg, struct sig_dbg_op __user *dbg);
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
- struct __kernel_old_timeval __user *tvp);
unsigned long __init early_init(unsigned long dt_ptr);
void __init machine_init(u64 dt_ptr);
#endif
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..70b0eb5bedfd 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -71,31 +71,6 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
}
-#ifdef CONFIG_PPC32
-/*
- * Due to some executables calling the wrong select we sometimes
- * get wrong args. This determines how the args are being passed
- * (a single ptr to them all args passed) then calls
- * sys_select() with the appropriate args. -- Cort
- */
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
-{
- if ( (unsigned long)n >= 4096 )
- {
- unsigned long __user *buffer = (unsigned long __user *)n;
- if (!access_ok(buffer, 5*sizeof(unsigned long))
- || __get_user(n, buffer)
- || __get_user(inp, ((fd_set __user * __user *)(buffer+1)))
- || __get_user(outp, ((fd_set __user * __user *)(buffer+2)))
- || __get_user(exp, ((fd_set __user * __user *)(buffer+3)))
- || __get_user(tvp, ((struct __kernel_old_timeval __user * __user *)(buffer+4))))
- return -EFAULT;
- }
- return sys_select(n, inp, outp, exp, tvp);
-}
-#endif
-
#ifdef CONFIG_PPC64
long ppc64_personality(unsigned long personality)
{
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 0b2480cf3e47..5bb0e90e502e 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -110,9 +110,7 @@
79 common settimeofday sys_settimeofday compat_sys_settimeofday
80 common getgroups sys_getgroups
81 common setgroups sys_setgroups
-82 32 select ppc_select sys_ni_syscall
-82 64 select sys_ni_syscall
-82 spu select sys_ni_syscall
+82 common select sys_ni_syscall
83 common symlink sys_symlink
84 32 oldlstat sys_lstat sys_ni_syscall
84 64 oldlstat sys_ni_syscall
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] powerpc: sstep: Fix load and update emulation
From: Naveen N. Rao @ 2021-03-03 16:31 UTC (permalink / raw)
To: Segher Boessenkool
Cc: ravi.bangoria, ananth, jniethe5, paulus, Sandipan Das,
linuxppc-dev, dja
In-Reply-To: <20210302023732.GH29191@gate.crashing.org>
On 2021/03/01 08:37PM, Segher Boessenkool wrote:
> Hi!
>
> I didn't see this until now, almost a month later, sorry about that :-)
No problem.
>
> On Thu, Feb 04, 2021 at 01:57:53PM +0530, Naveen N. Rao wrote:
> > On 2021/02/03 03:17PM, Segher Boessenkool wrote:
> > > Power8 does:
> > >
> > > Load with Update Instructions (RA = 0)
> > > EA is placed into R0.
> > > Load with Update Instructions (RA = RT)
> > > EA is placed into RT. The storage operand addressed by EA is
> > > accessed, but the data returned by the load is discarded.
> >
> > I'm actually not seeing that. This is what I am testing with:
> > li 8,0xaaa
> > mr 6,1
> > std 8,64(6)
> > #ldu 6,64(6)
> > .long 0xe8c60041
> >
> > And, r6 always ends up with 0xaea. It changes with the value I put into
> > r6 though.
>
> That is exactly the behaviour specified for p8. 0aaa+0040=0aea.
>
> > Granted, this is all up in the air, but it does look like there is more
> > going on and the value isn't the EA or the value at the address.
>
> That *is* the EA. The EA is the address the insn does the access at.
I'm probably missing something here. 0xaaa is the value I stored at an
offset of 64 bytes from the stack pointer (r1 is copied into r6). In the
ldu instruction above, the EA is 64(r6), which should translate to
r1+64. The data returned by the load would be 0xaaa, which should be
discarded per the description you provided above. So, I would expect to
see a 0xc0.. address in r6.
In fact, this looks to be the behavior documented for P9:
> > Power9 does:
> >
> > Load with Update Instructions (RA = 0)
> > EA is placed into R0.
> > Load with Update Instructions (RA = RT)
> > The storage operand addressed by EA is accessed. The
> > displacement
> > field is added to the data returned by the load and placed into
> > RT.
- Naveen
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-04 14:08 UTC (permalink / raw)
To: Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <YEDXJ5JNkgvDFehc@elver.google.com>
Le 04/03/2021 à 13:48, Marco Elver a écrit :
> From d118080eb9552073f5dcf1f86198f3d86d5ea850 Mon Sep 17 00:00:00 2001
> From: Marco Elver <elver@google.com>
> Date: Thu, 4 Mar 2021 13:15:51 +0100
> Subject: [PATCH] kfence: fix reports if constant function prefixes exist
>
> Some architectures prefix all functions with a constant string ('.' on
> ppc64). Add ARCH_FUNC_PREFIX, which may optionally be defined in
> <asm/kfence.h>, so that get_stack_skipnr() can work properly.
It works, thanks.
>
> Link: https://lkml.kernel.org/r/f036c53d-7e81-763c-47f4-6024c6c5f058@csgroup.eu
> Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Marco Elver <elver@google.com>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> mm/kfence/report.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/mm/kfence/report.c b/mm/kfence/report.c
> index 519f037720f5..e3f71451ad9e 100644
> --- a/mm/kfence/report.c
> +++ b/mm/kfence/report.c
> @@ -20,6 +20,11 @@
>
> #include "kfence.h"
>
> +/* May be overridden by <asm/kfence.h>. */
> +#ifndef ARCH_FUNC_PREFIX
> +#define ARCH_FUNC_PREFIX ""
> +#endif
> +
> extern bool no_hash_pointers;
>
> /* Helper function to either print to a seq_file or to console. */
> @@ -67,8 +72,9 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
> for (skipnr = 0; skipnr < num_entries; skipnr++) {
> int len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skipnr]);
>
> - if (str_has_prefix(buf, "kfence_") || str_has_prefix(buf, "__kfence_") ||
> - !strncmp(buf, "__slab_free", len)) {
> + if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfence_") ||
> + str_has_prefix(buf, ARCH_FUNC_PREFIX "__kfence_") ||
> + !strncmp(buf, ARCH_FUNC_PREFIX "__slab_free", len)) {
> /*
> * In case of tail calls from any of the below
> * to any of the above.
> @@ -77,10 +83,10 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
> }
>
> /* Also the *_bulk() variants by only checking prefixes. */
> - if (str_has_prefix(buf, "kfree") ||
> - str_has_prefix(buf, "kmem_cache_free") ||
> - str_has_prefix(buf, "__kmalloc") ||
> - str_has_prefix(buf, "kmem_cache_alloc"))
> + if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfree") ||
> + str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_free") ||
> + str_has_prefix(buf, ARCH_FUNC_PREFIX "__kmalloc") ||
> + str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_alloc"))
> goto found;
> }
> if (fallback < num_entries)
>
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Marco Elver @ 2021-03-04 14:19 UTC (permalink / raw)
To: Christophe Leroy
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <4b46ecc9-ae47-eee1-843e-e0638a356b51@csgroup.eu>
On Thu, 4 Mar 2021 at 15:08, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 04/03/2021 à 13:48, Marco Elver a écrit :
> > From d118080eb9552073f5dcf1f86198f3d86d5ea850 Mon Sep 17 00:00:00 2001
> > From: Marco Elver <elver@google.com>
> > Date: Thu, 4 Mar 2021 13:15:51 +0100
> > Subject: [PATCH] kfence: fix reports if constant function prefixes exist
> >
> > Some architectures prefix all functions with a constant string ('.' on
> > ppc64). Add ARCH_FUNC_PREFIX, which may optionally be defined in
> > <asm/kfence.h>, so that get_stack_skipnr() can work properly.
>
>
> It works, thanks.
>
> >
> > Link: https://lkml.kernel.org/r/f036c53d-7e81-763c-47f4-6024c6c5f058@csgroup.eu
> > Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > Signed-off-by: Marco Elver <elver@google.com>
>
> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Thanks, I'll send this to Andrew for inclusion in -mm, since this is
not a strict dependency (it'll work without the patch, just the stack
traces aren't that pretty but still useful). If the ppc patches and
this make it into the next merge window, everything should be good for
5.13.
> > ---
> > mm/kfence/report.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/kfence/report.c b/mm/kfence/report.c
> > index 519f037720f5..e3f71451ad9e 100644
> > --- a/mm/kfence/report.c
> > +++ b/mm/kfence/report.c
> > @@ -20,6 +20,11 @@
> >
> > #include "kfence.h"
> >
> > +/* May be overridden by <asm/kfence.h>. */
> > +#ifndef ARCH_FUNC_PREFIX
> > +#define ARCH_FUNC_PREFIX ""
> > +#endif
> > +
> > extern bool no_hash_pointers;
> >
> > /* Helper function to either print to a seq_file or to console. */
> > @@ -67,8 +72,9 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
> > for (skipnr = 0; skipnr < num_entries; skipnr++) {
> > int len = scnprintf(buf, sizeof(buf), "%ps", (void *)stack_entries[skipnr]);
> >
> > - if (str_has_prefix(buf, "kfence_") || str_has_prefix(buf, "__kfence_") ||
> > - !strncmp(buf, "__slab_free", len)) {
> > + if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfence_") ||
> > + str_has_prefix(buf, ARCH_FUNC_PREFIX "__kfence_") ||
> > + !strncmp(buf, ARCH_FUNC_PREFIX "__slab_free", len)) {
> > /*
> > * In case of tail calls from any of the below
> > * to any of the above.
> > @@ -77,10 +83,10 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
> > }
> >
> > /* Also the *_bulk() variants by only checking prefixes. */
> > - if (str_has_prefix(buf, "kfree") ||
> > - str_has_prefix(buf, "kmem_cache_free") ||
> > - str_has_prefix(buf, "__kmalloc") ||
> > - str_has_prefix(buf, "kmem_cache_alloc"))
> > + if (str_has_prefix(buf, ARCH_FUNC_PREFIX "kfree") ||
> > + str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_free") ||
> > + str_has_prefix(buf, ARCH_FUNC_PREFIX "__kmalloc") ||
> > + str_has_prefix(buf, ARCH_FUNC_PREFIX "kmem_cache_alloc"))
> > goto found;
> > }
> > if (fallback < num_entries)
> >
^ permalink raw reply
* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
From: Rob Herring @ 2021-03-04 14:32 UTC (permalink / raw)
To: Daniel Walker
Cc: devicetree, Ruslan Ruslichenko, Daniel Gimpelevich, Frank Rowand,
linuxppc-dev, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, xe-linux-external, Andrew Morton,
Will Deacon
In-Reply-To: <20210304044803.812204-2-danielwa@cisco.com>
On Wed, Mar 3, 2021 at 10:48 PM Daniel Walker <danielwa@cisco.com> wrote:
>
> It looks like there's some seepage of cmdline stuff into
> the generic device tree code. This conflicts with the
> generic cmdline implementation so I remove it in the case
> when that's enabled.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> drivers/of/fdt.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index feb0f2d67fc5..cfe4f8d3c9f5 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -25,6 +25,7 @@
> #include <linux/serial_core.h>
> #include <linux/sysfs.h>
> #include <linux/random.h>
> +#include <linux/cmdline.h>
>
> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> #include <asm/page.h>
> @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
> early_init_dt_check_for_initrd(node);
>
> +#ifdef CONFIG_GENERIC_CMDLINE
What I like about Christophe's version is it removes the old DT
implementation. Who's going to convert the rest of the DT based
arches? That's arm, arm64, hexagon, microblaze, nios2, openrisc,
riscv, sh, and xtensa. Either separate the common code from the config
like Christophe's version or these all need converting. Though it's
fine to hash out patch 1 with a couple of arches first.
> /* Retrieve command line */
> p = of_get_flat_dt_prop(node, "bootargs", &l);
This needs to be outside the ifdef.
> +
> + /*
> + * The builtin command line will be added here, or it can override
> + * with the DT bootargs.
> + */
> + cmdline_add_builtin(data,
> + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
> + COMMAND_LINE_SIZE);
> +#else
> if (p != NULL && l > 0)
> strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
>
> @@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> #endif
> #endif /* CONFIG_CMDLINE */
> +#endif /* CONFIG_GENERIC_CMDLINE */
>
> pr_debug("Command line is: %s\n", (char *)data);
>
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH v2 3/4] powerpc/64s: Allow double call of kernel_[un]map_linear_page()
From: Christophe Leroy @ 2021-03-04 14:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Alexander Potapenko, Marco Elver, Dmitry Vyukov
Cc: linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <8dfe1bd2abde26337c1d8c1ad0acfcc82185e0d5.1614868445.git.christophe.leroy@csgroup.eu>
If the page is already mapped resp. already unmapped, bail out.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: New
---
arch/powerpc/mm/book3s64/hash_utils.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index f1b5a5f1d3a9..cb09a49be798 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1944,6 +1944,9 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
if (!vsid)
return;
+ if (linear_map_hash_slots[lmi] & 0x80)
+ return;
+
ret = hpte_insert_repeating(hash, vpn, __pa(vaddr), mode,
HPTE_V_BOLTED,
mmu_linear_psize, mmu_kernel_ssize);
@@ -1963,7 +1966,10 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
spin_lock(&linear_map_hash_lock);
- BUG_ON(!(linear_map_hash_slots[lmi] & 0x80));
+ if (!(linear_map_hash_slots[lmi] & 0x80)) {
+ spin_unlock(&linear_map_hash_lock);
+ return;
+ }
hidx = linear_map_hash_slots[lmi] & 0x7f;
linear_map_hash_slots[lmi] = 0;
spin_unlock(&linear_map_hash_lock);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 2/4] powerpc/64s: Remove unneeded #ifdef CONFIG_DEBUG_PAGEALLOC in hash_utils
From: Christophe Leroy @ 2021-03-04 14:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Alexander Potapenko, Marco Elver, Dmitry Vyukov
Cc: linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <8dfe1bd2abde26337c1d8c1ad0acfcc82185e0d5.1614868445.git.christophe.leroy@csgroup.eu>
debug_pagealloc_enabled() is always defined and constant folds to
'false' when CONFIG_DEBUG_PAGEALLOC is not enabled.
Remove the #ifdefs, the code and associated static variables will
be optimised out by the compiler when CONFIG_DEBUG_PAGEALLOC is
not defined.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: New
---
arch/powerpc/mm/book3s64/hash_utils.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 581b20a2feaf..f1b5a5f1d3a9 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -126,11 +126,8 @@ EXPORT_SYMBOL_GPL(mmu_slb_size);
#ifdef CONFIG_PPC_64K_PAGES
int mmu_ci_restrictions;
#endif
-#ifdef CONFIG_DEBUG_PAGEALLOC
static u8 *linear_map_hash_slots;
static unsigned long linear_map_hash_count;
-static DEFINE_SPINLOCK(linear_map_hash_lock);
-#endif /* CONFIG_DEBUG_PAGEALLOC */
struct mmu_hash_ops mmu_hash_ops;
EXPORT_SYMBOL(mmu_hash_ops);
@@ -326,11 +323,9 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
break;
cond_resched();
-#ifdef CONFIG_DEBUG_PAGEALLOC
if (debug_pagealloc_enabled() &&
(paddr >> PAGE_SHIFT) < linear_map_hash_count)
linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
-#endif /* CONFIG_DEBUG_PAGEALLOC */
}
return ret < 0 ? ret : 0;
}
@@ -954,7 +949,6 @@ static void __init htab_initialize(void)
prot = pgprot_val(PAGE_KERNEL);
-#ifdef CONFIG_DEBUG_PAGEALLOC
if (debug_pagealloc_enabled()) {
linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
linear_map_hash_slots = memblock_alloc_try_nid(
@@ -964,7 +958,6 @@ static void __init htab_initialize(void)
panic("%s: Failed to allocate %lu bytes max_addr=%pa\n",
__func__, linear_map_hash_count, &ppc64_rma_size);
}
-#endif /* CONFIG_DEBUG_PAGEALLOC */
/* create bolted the linear mapping in the hash table */
for_each_mem_range(i, &base, &end) {
@@ -1935,6 +1928,8 @@ long hpte_insert_repeating(unsigned long hash, unsigned long vpn,
}
#ifdef CONFIG_DEBUG_PAGEALLOC
+static DEFINE_SPINLOCK(linear_map_hash_lock);
+
static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
{
unsigned long hash;
--
2.25.0
^ permalink raw reply related
* [PATCH v2 1/4] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-04 14:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Alexander Potapenko, Marco Elver, Dmitry Vyukov
Cc: linuxppc-dev, linux-kernel, kasan-dev
Add architecture specific implementation details for KFENCE and enable
KFENCE for the ppc32 architecture. In particular, this implements the
required interface in <asm/kfence.h>.
KFENCE requires that attributes for pages from its memory pool can
individually be set. Therefore, force the Read/Write linear map to be
mapped at page granularity.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Marco Elver <elver@google.com>
---
v2: Added debug_pagealloc_enabled_or_kfence()
---
arch/powerpc/Kconfig | 13 ++++++------
arch/powerpc/include/asm/kfence.h | 33 +++++++++++++++++++++++++++++++
arch/powerpc/mm/book3s32/mmu.c | 2 +-
arch/powerpc/mm/fault.c | 7 ++++++-
arch/powerpc/mm/init_32.c | 3 +++
arch/powerpc/mm/mmu_decl.h | 5 +++++
arch/powerpc/mm/nohash/8xx.c | 4 ++--
7 files changed, 57 insertions(+), 10 deletions(-)
create mode 100644 arch/powerpc/include/asm/kfence.h
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 386ae12d8523..d46db0bfb998 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,6 +185,7 @@ config PPC
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
select HAVE_ARCH_KGDB
+ select HAVE_ARCH_KFENCE if PPC32
select HAVE_ARCH_MMAP_RND_BITS
select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
select HAVE_ARCH_NVRAM_OPS
@@ -786,7 +787,7 @@ config THREAD_SHIFT
config DATA_SHIFT_BOOL
bool "Set custom data alignment"
depends on ADVANCED_OPTIONS
- depends on STRICT_KERNEL_RWX || DEBUG_PAGEALLOC
+ depends on STRICT_KERNEL_RWX || DEBUG_PAGEALLOC || KFENCE
depends on PPC_BOOK3S_32 || (PPC_8xx && !PIN_TLB_DATA && !STRICT_KERNEL_RWX)
help
This option allows you to set the kernel data alignment. When
@@ -798,13 +799,13 @@ config DATA_SHIFT_BOOL
config DATA_SHIFT
int "Data shift" if DATA_SHIFT_BOOL
default 24 if STRICT_KERNEL_RWX && PPC64
- range 17 28 if (STRICT_KERNEL_RWX || DEBUG_PAGEALLOC) && PPC_BOOK3S_32
- range 19 23 if (STRICT_KERNEL_RWX || DEBUG_PAGEALLOC) && PPC_8xx
+ range 17 28 if (STRICT_KERNEL_RWX || DEBUG_PAGEALLOC || KFENCE) && PPC_BOOK3S_32
+ range 19 23 if (STRICT_KERNEL_RWX || DEBUG_PAGEALLOC || KFENCE) && PPC_8xx
default 22 if STRICT_KERNEL_RWX && PPC_BOOK3S_32
- default 18 if DEBUG_PAGEALLOC && PPC_BOOK3S_32
+ default 18 if (DEBUG_PAGEALLOC || KFENCE) && PPC_BOOK3S_32
default 23 if STRICT_KERNEL_RWX && PPC_8xx
- default 23 if DEBUG_PAGEALLOC && PPC_8xx && PIN_TLB_DATA
- default 19 if DEBUG_PAGEALLOC && PPC_8xx
+ default 23 if (DEBUG_PAGEALLOC || KFENCE) && PPC_8xx && PIN_TLB_DATA
+ default 19 if (DEBUG_PAGEALLOC || KFENCE) && PPC_8xx
default PPC_PAGE_SHIFT
help
On Book3S 32 (603+), DBATs are used to map kernel text and rodata RO.
diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h
new file mode 100644
index 000000000000..a9846b68c6b9
--- /dev/null
+++ b/arch/powerpc/include/asm/kfence.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * powerpc KFENCE support.
+ *
+ * Copyright (C) 2020 CS GROUP France
+ */
+
+#ifndef __ASM_POWERPC_KFENCE_H
+#define __ASM_POWERPC_KFENCE_H
+
+#include <linux/mm.h>
+#include <asm/pgtable.h>
+
+static inline bool arch_kfence_init_pool(void)
+{
+ return true;
+}
+
+static inline bool kfence_protect_page(unsigned long addr, bool protect)
+{
+ pte_t *kpte = virt_to_kpte(addr);
+
+ if (protect) {
+ pte_update(&init_mm, addr, kpte, _PAGE_PRESENT, 0, 0);
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+ } else {
+ pte_update(&init_mm, addr, kpte, 0, _PAGE_PRESENT, 0);
+ }
+
+ return true;
+}
+
+#endif /* __ASM_POWERPC_KFENCE_H */
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index d7eb266a3f7a..a0db398b5c26 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -162,7 +162,7 @@ unsigned long __init mmu_mapin_ram(unsigned long base, unsigned long top)
unsigned long border = (unsigned long)__init_begin - PAGE_OFFSET;
- if (debug_pagealloc_enabled() || __map_without_bats) {
+ if (debug_pagealloc_enabled_or_kfence() || __map_without_bats) {
pr_debug_once("Read-Write memory mapped without BATs\n");
if (base >= border)
return base;
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index bb368257b55c..bea13682c909 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -32,6 +32,7 @@
#include <linux/context_tracking.h>
#include <linux/hugetlb.h>
#include <linux/uaccess.h>
+#include <linux/kfence.h>
#include <asm/firmware.h>
#include <asm/interrupt.h>
@@ -418,8 +419,12 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
* take a page fault to a kernel address or a page fault to a user
* address outside of dedicated places
*/
- if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write)))
+ if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write))) {
+ if (kfence_handle_page_fault(address, is_write, regs))
+ return 0;
+
return SIGSEGV;
+ }
/*
* If we're in an interrupt, have no user context or are running
diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 02c7db4087cb..3d690be48e84 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -97,6 +97,9 @@ static void __init MMU_setup(void)
if (IS_ENABLED(CONFIG_PPC_8xx))
return;
+ if (IS_ENABLED(CONFIG_KFENCE))
+ __map_without_ltlbs = 1;
+
if (debug_pagealloc_enabled())
__map_without_ltlbs = 1;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 998810e68562..7dac910c0b21 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -185,3 +185,8 @@ void ptdump_check_wx(void);
#else
static inline void ptdump_check_wx(void) { }
#endif
+
+static inline bool debug_pagealloc_enabled_or_kfence(void)
+{
+ return IS_ENABLED(CONFIG_KFENCE) || debug_pagealloc_enabled();
+}
diff --git a/arch/powerpc/mm/nohash/8xx.c b/arch/powerpc/mm/nohash/8xx.c
index 19a3eec1d8c5..71bfdbedacee 100644
--- a/arch/powerpc/mm/nohash/8xx.c
+++ b/arch/powerpc/mm/nohash/8xx.c
@@ -149,7 +149,7 @@ unsigned long __init mmu_mapin_ram(unsigned long base, unsigned long top)
{
unsigned long etext8 = ALIGN(__pa(_etext), SZ_8M);
unsigned long sinittext = __pa(_sinittext);
- bool strict_boundary = strict_kernel_rwx_enabled() || debug_pagealloc_enabled();
+ bool strict_boundary = strict_kernel_rwx_enabled() || debug_pagealloc_enabled_or_kfence();
unsigned long boundary = strict_boundary ? sinittext : etext8;
unsigned long einittext8 = ALIGN(__pa(_einittext), SZ_8M);
@@ -161,7 +161,7 @@ unsigned long __init mmu_mapin_ram(unsigned long base, unsigned long top)
return 0;
mmu_mapin_ram_chunk(0, boundary, PAGE_KERNEL_TEXT, true);
- if (debug_pagealloc_enabled()) {
+ if (debug_pagealloc_enabled_or_kfence()) {
top = boundary;
} else {
mmu_mapin_ram_chunk(boundary, einittext8, PAGE_KERNEL_TEXT, true);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 4/4] powerpc: Enable KFENCE on BOOK3S/64
From: Christophe Leroy @ 2021-03-04 14:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Alexander Potapenko, Marco Elver, Dmitry Vyukov
Cc: linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <8dfe1bd2abde26337c1d8c1ad0acfcc82185e0d5.1614868445.git.christophe.leroy@csgroup.eu>
This reuses the DEBUG_PAGEALLOC logic.
Tested on qemu with ppc64_defconfig + CONFIG_KFENCE + CONFIG_KUNIT +
CONFIG_KFENCE_KUNIT_TEST.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: New
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/kfence.h | 8 ++++++++
arch/powerpc/mm/book3s64/hash_utils.c | 29 +++++++++++++++++++++------
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d46db0bfb998..67c47b60cc84 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,7 +185,7 @@ config PPC
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
select HAVE_ARCH_KGDB
- select HAVE_ARCH_KFENCE if PPC32
+ select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC
select HAVE_ARCH_MMAP_RND_BITS
select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
select HAVE_ARCH_NVRAM_OPS
diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h
index a9846b68c6b9..532cc1a92fa5 100644
--- a/arch/powerpc/include/asm/kfence.h
+++ b/arch/powerpc/include/asm/kfence.h
@@ -11,11 +11,18 @@
#include <linux/mm.h>
#include <asm/pgtable.h>
+#if defined(CONFIG_PPC64) && !defined(PPC64_ELF_ABI_v2)
+#define ARCH_FUNC_PREFIX "."
+#endif
+
static inline bool arch_kfence_init_pool(void)
{
return true;
}
+#ifdef CONFIG_PPC64
+bool kfence_protect_page(unsigned long addr, bool protect);
+#else
static inline bool kfence_protect_page(unsigned long addr, bool protect)
{
pte_t *kpte = virt_to_kpte(addr);
@@ -29,5 +36,6 @@ static inline bool kfence_protect_page(unsigned long addr, bool protect)
return true;
}
+#endif
#endif /* __ASM_POWERPC_KFENCE_H */
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index cb09a49be798..b967a6403e59 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -323,8 +323,8 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
break;
cond_resched();
- if (debug_pagealloc_enabled() &&
- (paddr >> PAGE_SHIFT) < linear_map_hash_count)
+ if (debug_pagealloc_enabled_or_kfence() &&
+ (paddr >> PAGE_SHIFT) < linear_map_hash_count)
linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
}
return ret < 0 ? ret : 0;
@@ -661,7 +661,7 @@ static void __init htab_init_page_sizes(void)
bool aligned = true;
init_hpte_page_sizes();
- if (!debug_pagealloc_enabled()) {
+ if (!debug_pagealloc_enabled_or_kfence()) {
/*
* Pick a size for the linear mapping. Currently, we only
* support 16M, 1M and 4K which is the default
@@ -949,7 +949,7 @@ static void __init htab_initialize(void)
prot = pgprot_val(PAGE_KERNEL);
- if (debug_pagealloc_enabled()) {
+ if (debug_pagealloc_enabled_or_kfence()) {
linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
linear_map_hash_slots = memblock_alloc_try_nid(
linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,
@@ -1927,7 +1927,7 @@ long hpte_insert_repeating(unsigned long hash, unsigned long vpn,
return slot;
}
-#ifdef CONFIG_DEBUG_PAGEALLOC
+#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
static DEFINE_SPINLOCK(linear_map_hash_lock);
static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
@@ -1982,6 +1982,21 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
mmu_kernel_ssize, 0);
}
+#ifdef CONFIG_KFENCE
+bool kfence_protect_page(unsigned long addr, bool protect)
+{
+ unsigned long lmi = __pa(addr) >> PAGE_SHIFT;
+
+ if (protect)
+ kernel_unmap_linear_page(addr, lmi);
+ else
+ kernel_map_linear_page(addr, lmi);
+
+ return true;
+}
+#endif
+
+#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
unsigned long flags, vaddr, lmi;
@@ -2000,7 +2015,9 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
}
local_irq_restore(flags);
}
-#endif /* CONFIG_DEBUG_PAGEALLOC */
+#endif
+
+#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_KFENCE */
void hash__setup_initial_memory_limit(phys_addr_t first_memblock_base,
phys_addr_t first_memblock_size)
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
From: Mark Rutland @ 2021-03-04 14:57 UTC (permalink / raw)
To: Marco Elver
Cc: Catalin Marinas, Will Deacon, LKML, broonie, Paul Mackerras,
kasan-dev, linuxppc-dev, linux-arm-kernel
In-Reply-To: <YD+o5QkCZN97mH8/@elver.google.com>
[adding Mark Brown]
On Wed, Mar 03, 2021 at 04:20:43PM +0100, Marco Elver wrote:
> On Wed, Mar 03, 2021 at 03:52PM +0100, Christophe Leroy wrote:
> > Le 03/03/2021 � 15:38, Marco Elver a �crit�:
> > > On Wed, 3 Mar 2021 at 15:09, Christophe Leroy
> > > <christophe.leroy@csgroup.eu> wrote:
> > > >
> > > > It seems like all other sane architectures, namely x86 and arm64
> > > > at least, include the running function as top entry when saving
> > > > stack trace.
> > > >
> > > > Functionnalities like KFENCE expect it.
> > > >
> > > > Do the same on powerpc, it allows KFENCE to properly identify the faulting
> > > > function as depicted below. Before the patch KFENCE was identifying
> > > > finish_task_switch.isra as the faulting function.
> > > >
> > > > [ 14.937370] ==================================================================
> > > > [ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
> > > > [ 14.948692]
> > > > [ 14.956814] Invalid read at 0xdf98800a:
> > > > [ 14.960664] test_invalid_access+0x54/0x108
> > > > [ 14.964876] finish_task_switch.isra.0+0x54/0x23c
> > > > [ 14.969606] kunit_try_run_case+0x5c/0xd0
> > > > [ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
> > > > [ 14.979079] kthread+0x15c/0x174
> > > > [ 14.982342] ret_from_kernel_thread+0x14/0x1c
> > > > [ 14.986731]
> > > > [ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B 5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
> > > > [ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
> > > > [ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: G B (5.12.0-rc1-01537-g95f6e2088d7e-dirty)
> > > > [ 15.015274] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 22000004 XER: 00000000
> > > > [ 15.022043] DAR: df98800a DSISR: 20000000
> > > > [ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 00000008 c084b32b c016ebd8
> > > > [ 15.022043] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288
> > > > [ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
> > > > [ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
> > > > [ 15.051181] Call Trace:
> > > > [ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c (unreliable)
> > > > [ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
> > > > [ 15.067215] [e2449ed0] [c02f648c] kunit_generic_run_threadfn_adapter+0x24/0x30
> > > > [ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
> > > > [ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
> > > > [ 15.085798] Instruction dump:
> > > > [ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 39000000 995f0024 907f0028 90ff001c
> > > > [ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f
> > > > [ 15.104612] ==================================================================
> > > >
> > > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > >
> > > Acked-by: Marco Elver <elver@google.com>
> > >
> > > Thank you, I think this looks like the right solution. Just a question below:
> > >
> > ...
> >
> > > > @@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace)
> > > >
> > > > sp = current_stack_frame();
> > > >
> > > > - save_context_stack(trace, sp, current, 1);
> > > > + save_context_stack(trace, sp, (unsigned long)save_stack_trace, current, 1);
> > >
> > > This causes ip == save_stack_trace and also below for
> > > save_stack_trace_tsk. Does this mean save_stack_trace() is included in
> > > the trace? Looking at kernel/stacktrace.c, I think the library wants
> > > to exclude itself from the trace, as it does '.skip = skipnr + 1' (and
> > > '.skip = skipnr + (current == tsk)' for the _tsk variant).
> > >
> > > If the arch-helper here is included, should this use _RET_IP_ instead?
> > >
> >
> > Don't really know, I was inspired by arm64 which has:
> >
> > void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
> > struct task_struct *task, struct pt_regs *regs)
> > {
> > struct stackframe frame;
> >
> > if (regs)
> > start_backtrace(&frame, regs->regs[29], regs->pc);
> > else if (task == current)
> > start_backtrace(&frame,
> > (unsigned long)__builtin_frame_address(0),
> > (unsigned long)arch_stack_walk);
> > else
> > start_backtrace(&frame, thread_saved_fp(task),
> > thread_saved_pc(task));
> >
> > walk_stackframe(task, &frame, consume_entry, cookie);
> > }
> >
> > But looking at x86 you may be right, so what should be done really ?
>
> x86:
>
> [ 2.843292] calling stack_trace_save:
> [ 2.843705] test_func+0x6c/0x118
> [ 2.844184] do_one_initcall+0x58/0x270
> [ 2.844618] kernel_init_freeable+0x1da/0x23a
> [ 2.845110] kernel_init+0xc/0x166
> [ 2.845494] ret_from_fork+0x22/0x30
>
> [ 2.867525] calling stack_trace_save_tsk:
> [ 2.868017] test_func+0xa9/0x118
> [ 2.868530] do_one_initcall+0x58/0x270
> [ 2.869003] kernel_init_freeable+0x1da/0x23a
> [ 2.869535] kernel_init+0xc/0x166
> [ 2.869957] ret_from_fork+0x22/0x30
>
> arm64:
>
> [ 3.786911] calling stack_trace_save:
> [ 3.787147] stack_trace_save+0x50/0x78
> [ 3.787443] test_func+0x84/0x13c
> [ 3.787738] do_one_initcall+0x5c/0x310
> [ 3.788099] kernel_init_freeable+0x214/0x294
> [ 3.788363] kernel_init+0x18/0x164
> [ 3.788585] ret_from_fork+0x10/0x30
>
> [ 3.803615] calling stack_trace_save_tsk:
> [ 3.804266] stack_trace_save_tsk+0x9c/0x100
> [ 3.804541] test_func+0xc4/0x13c
> [ 3.804803] do_one_initcall+0x5c/0x310
> [ 3.805031] kernel_init_freeable+0x214/0x294
> [ 3.805284] kernel_init+0x18/0x164
> [ 3.805505] ret_from_fork+0x10/0x30
>
> +Cc arm64 folks.
>
> So I think the arm64 version also has a bug, because I think a user of
> <linux/stacktrace.h> really doesn't care about the library function
> itself. And from reading kernel/stacktrace.c I think it wants to exclude
> itself entirely.
>
> It's a shame that <linux/stacktrace.h> isn't better documented, but I'm
> pretty sure that including the library functions in the trace is not
> useful.
I agree this behaviour isn't desireable, and that the lack of
documentation is unfortunate.
It looks like GCC is happy to give us the function-entry-time FP if we use
__builtin_frame_address(1), and assuming clang is similarly happy we can do:
| diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
| index ad20981dfda4..5dfbf915eb7f 100644
| --- a/arch/arm64/kernel/stacktrace.c
| +++ b/arch/arm64/kernel/stacktrace.c
| @@ -203,8 +203,8 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
| start_backtrace(&frame, regs->regs[29], regs->pc);
| else if (task == current)
| start_backtrace(&frame,
| - (unsigned long)__builtin_frame_address(0),
| - (unsigned long)arch_stack_walk);
| + (unsigned long)__builtin_frame_address(1),
| + (unsigned long)__builtin_return_address(0));
| else
| start_backtrace(&frame, thread_saved_fp(task),
| thread_saved_pc(task));
... such that arch_stack_walk() will try to avoid including itself in a
trace, and so the existing skipping should (w/ caveats below) skip
stack_trace_save() or stack_trace_save_tsk().
If that works for you, I can spin that as a patch, though we'll need to
check that doesn't introduce a new fencepost error elsewhere.
The bigger problem here is that skipping is dodgy to begin with, and
this is still liable to break in some cases. One big concern is that
(especially with LTO) we cannot guarantee the compiler will not inline
or outline functions, causing the skipp value to be too large or too
small. That's liable to happen to callers, and in theory (though
unlikely in practice), portions of arch_stack_walk() or
stack_trace_save() could get outlined too.
Unless we can get some strong guarantees from compiler folk such that we
can guarantee a specific function acts boundary for unwinding (and
doesn't itself get split, etc), the only reliable way I can think to
solve this requires an assembly trampoline. Whatever we do is liable to
need some invasive rework.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v2] powerpc/32: remove bogus ppc_select syscall
From: Arnd Bergmann @ 2021-03-04 15:17 UTC (permalink / raw)
To: Christophe Leroy
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Paul Mackerras,
halesh.sadashiv
In-Reply-To: <2daa39fa210d971863a6f9ac7c81849764e7a261.1614862233.git.christophe.leroy@csgroup.eu>
On Thu, Mar 4, 2021 at 1:51 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The ppc_select function was introduced in linux-2.3.48 in order to support
> code confusing the legacy select() calling convention with the standard one.
> Even 24 years ago, all correctly built code should not have done this and
> could have easily been phased out. Nothing that was compiled later should
> actually try to use the old_select interface, and it would have been broken
> already on all ppc64 kernels with the syscall emulation layer.
>
> This patch brings the 32 bit compat ABI and the native 32 bit ABI for
> powerpc into a consistent state, by removing support for both the
> old_select system call number and the handler for it.
>
> The bug report triggering this came from
> Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>, who discovered that the
> 32 bit implementation of ppc_select would in case of a negative number
> of file descriptors incorrectly return -EFAULT instead of -EINVAL.
> There seems to be no way to fix this problem in a way that would
> keep broken pre-1997 binaries running.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>
> [chleroy: Rebased and updated the number of years elapsed in the commit message]
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> First version was in 2008, at that time it was rejected, see
> http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/
The patch from 2008 did two things:
- it removed the ppc32 specific 'select' syscall at #82
- it fixed the generic '_newselect' syscall at #142
Back then, the decision was to only address the second issue, which
got merged in commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from
ppc32 select syscall").
It is probably ok to remove the old select system call now, but
my changelog text no longer makes sense, as the patch has nothing
to do with the bug that was reported back then.
Arnd
^ permalink raw reply
* Re: [PATCH v2] powerpc/32: remove bogus ppc_select syscall
From: Christophe Leroy @ 2021-03-04 15:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Paul Mackerras,
halesh.sadashiv
In-Reply-To: <CAK8P3a2=qOG1iLhw2fi=r128bRMdfNx4BseXONiS7vrnbVvr6w@mail.gmail.com>
Le 04/03/2021 à 16:17, Arnd Bergmann a écrit :
> On Thu, Mar 4, 2021 at 1:51 PM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The ppc_select function was introduced in linux-2.3.48 in order to support
>> code confusing the legacy select() calling convention with the standard one.
>> Even 24 years ago, all correctly built code should not have done this and
>> could have easily been phased out. Nothing that was compiled later should
>> actually try to use the old_select interface, and it would have been broken
>> already on all ppc64 kernels with the syscall emulation layer.
>>
>> This patch brings the 32 bit compat ABI and the native 32 bit ABI for
>> powerpc into a consistent state, by removing support for both the
>> old_select system call number and the handler for it.
>>
>> The bug report triggering this came from
>> Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>, who discovered that the
>> 32 bit implementation of ppc_select would in case of a negative number
>> of file descriptors incorrectly return -EFAULT instead of -EINVAL.
>> There seems to be no way to fix this problem in a way that would
>> keep broken pre-1997 binaries running.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Cc: Halesh Sadashiiv <halesh.sadashiv@ap.sony.com>
>> [chleroy: Rebased and updated the number of years elapsed in the commit message]
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> First version was in 2008, at that time it was rejected, see
>> http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/
>
> The patch from 2008 did two things:
>
> - it removed the ppc32 specific 'select' syscall at #82
> - it fixed the generic '_newselect' syscall at #142
>
> Back then, the decision was to only address the second issue, which
> got merged in commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from
> ppc32 select syscall").
>
> It is probably ok to remove the old select system call now, but
> my changelog text no longer makes sense, as the patch has nothing
> to do with the bug that was reported back then.
>
I understood that the original reported bug was that calling that version of select() with a
negative value as first parametre would lead to a -EFAULT instead of -EINVAL. That's exactly the
case here, if you set n = -1 you get into this (unsigned long)n > 4096 then the buffer is at
0xffffffff and access_ok() won't grand access to it so the return value will be -EFAULT instead of
-EINVAL.
Am I missing something ?
Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox