Re: [PATCH 1/2] powerpc/platforms/pseries: Fix STK_PARAM access in the hcall tracing code

2023-10-15 Thread Michael Ellerman
On Fri, 29 Sep 2023 22:53:36 +0530, Athira Rajeev wrote:
> In powerpc pseries system, below behaviour is observed while
> enabling tracing on hcall:
>   # cd /sys/kernel/debug/tracing/
>   # cat events/powerpc/hcall_exit/enable
>   0
>   # echo 1 > events/powerpc/hcall_exit/enable
> 
> [...]

Applied to powerpc/fixes.

[1/2] powerpc/platforms/pseries: Fix STK_PARAM access in the hcall tracing code
  https://git.kernel.org/powerpc/c/3b678768c0458e6d8d45fadf61423e44effed4cb
[2/2] powerpc/platforms/pseries: Remove unused r0 in the hcall tracing code
  https://git.kernel.org/powerpc/c/dfb5f8cbd5992d5769edfd3e059fad9e0b8bdafb

cheers


[PATCH 1/2] powerpc/platforms/pseries: Fix STK_PARAM access in the hcall tracing code

2023-09-29 Thread Athira Rajeev
In powerpc pseries system, below behaviour is observed while
enabling tracing on hcall:
# cd /sys/kernel/debug/tracing/
# cat events/powerpc/hcall_exit/enable
0
# echo 1 > events/powerpc/hcall_exit/enable

# ls
-bash: fork: Bad address

Above is from power9 lpar with latest kernel. Past this, softlockup
is observed. Initially while attempting via perf_event_open to
use "PERF_TYPE_TRACEPOINT", kernel panic was observed.

perf config used:

memset([1],0,sizeof(struct perf_event_attr));
pe[1].type=PERF_TYPE_TRACEPOINT;
pe[1].size=96;
pe[1].config=0x26ULL; /* 38 raw_syscalls/sys_exit */
pe[1].sample_type=0; /* 0 */

pe[1].read_format=PERF_FORMAT_TOTAL_TIME_ENABLED|PERF_FORMAT_TOTAL_TIME_RUNNING|PERF_FORMAT_ID|PERF_FORMAT_GROUP|0x10ULL;
 /* 1f */
pe[1].inherit=1;
pe[1].precise_ip=0; /* arbitrary skid */
pe[1].wakeup_events=0;
pe[1].bp_type=HW_BREAKPOINT_EMPTY;
pe[1].config1=0x1ULL;

Kernel panic logs:
==

Kernel attempted to read user page (8) - exploit attempt? (uid: 0)
 BUG: Kernel NULL pointer dereference on read at 0x0008
 Faulting instruction address: 0xc04c2814
 Oops: Kernel access of bad area, sig: 11 [#1]
 LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: nfnetlink bonding tls rfkill sunrpc dm_service_time 
dm_multipath pseries_rng xts vmx_crypto xfs libcrc32c sd_mod t10_pi 
crc64_rocksoft crc64 sg ibmvfc scsi_transport_fc ibmveth dm_mirror 
dm_region_hash dm_log dm_mod fuse
CPU: 0 PID: 1431 Comm: login Not tainted 6.4.0+ #1
Hardware name: IBM,8375-42A POWER9 (raw) 0x4e0202 0xf05 
of:IBM,FW950.30 (VL950_892) hv:phyp pSeries
NIP [c04c2814] page_remove_rmap+0x44/0x320
LR [c049c2a4] wp_page_copy+0x384/0xec0
Call Trace:
[c98c7ad0] [c0001416e400] 0xc0001416e400 (unreliable)
[c98c7b20] [c049c2a4] wp_page_copy+0x384/0xec0
[c98c7bf0] [c04a4f64] __handle_mm_fault+0x9d4/0xfb0
[c98c7cf0] [c04a5630] handle_mm_fault+0xf0/0x350
[c98c7d40] [c0094e8c] ___do_page_fault+0x48c/0xc90
[c98c7df0] [c00958a0] hash__do_page_fault+0x30/0x70
[c98c7e20] [c009e244] do_hash_fault+0x1a4/0x330
[c98c7e50] [c0008918] 
data_access_common_virt+0x198/0x1f0
 --- interrupt: 300 at 0x7fffae971abc

git bisect tracked this down to below commit:
'commit baa49d81a94b ("powerpc/pseries: hvcall stack frame overhead")'

This commit changed STACK_FRAME_OVERHEAD (112 ) to
STACK_FRAME_MIN_SIZE (32 ) since 32 bytes is the minimum size
for ELFv2 stack. With the latest kernel, when running on ELFv2,
STACK_FRAME_MIN_SIZE is used to allocate stack size.

During plpar_hcall_trace, first call is made to HCALL_INST_PRECALL
which saves the registers and allocates new stack frame. In the
plpar_hcall_trace code, STK_PARAM is accessed at two places.
1. To save r4: std r4,STK_PARAM(R4)(r1)
2. To access r4 back: ld  r12,STK_PARAM(R4)(r1)

HCALL_INST_PRECALL precall allocates a new stack frame. So all
the stack parameter access after the precall, needs to be accessed
with +STACK_FRAME_MIN_SIZE. So the store instruction should be:
std r4,STACK_FRAME_MIN_SIZE+STK_PARAM(R4)(r1)

If the "std" is not updated with STACK_FRAME_MIN_SIZE, we will
end up with overwriting stack contents and cause corruption.
But instead of updating 'std', we can instead remove it since
HCALL_INST_PRECALL already saves it to the correct location.

similarly load instruction should be:
ld  r12,STACK_FRAME_MIN_SIZE+STK_PARAM(R4)(r1)

Fix the load instruction to correctly access the stack parameter
with +STACK_FRAME_MIN_SIZE and remove the store of r4 since the
precall saves it correctly.

Cc: sta...@vger.kernel.org
Fixes: baa49d81a94b ("powerpc/pseries: hvcall stack frame overhead")
Co-developed-by: Naveen N Rao 
Signed-off-by: Naveen N Rao 
Signed-off-by: Athira Rajeev 
---
 arch/powerpc/platforms/pseries/hvCall.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hvCall.S 
b/arch/powerpc/platforms/pseries/hvCall.S
index bae45b358a09..2addf2ea03f0 100644
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -184,7 +184,6 @@ _GLOBAL_TOC(plpar_hcall)
 plpar_hcall_trace:
HCALL_INST_PRECALL(R5)
 
-   std r4,STK_PARAM(R4)(r1)
mr  r0,r4
 
mr  r4,r5
@@ -196,7 +195,7 @@ plpar_hcall_trace:
 
HVSC
 
-   ld  r12,STK_PARAM(R4)(r1)
+   ld  r12,STACK_FRAME_MIN_SIZE+STK_PARAM(R4)(r1)
std r4,0(r12)
std r5,8(r12)
std r6,16(r12)
@@ -296,7 +295,6 @@ _GLOBAL_TOC(plpar_hcall9)