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

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

[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(&pe[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 0x00000008
	 Faulting instruction address: 0xc0000000004c2814
	 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 0xf000005 of:IBM,FW950.30 (VL950_892) hv:phyp pSeries
 	NIP [c0000000004c2814] page_remove_rmap+0x44/0x320
	LR [c00000000049c2a4] wp_page_copy+0x384/0xec0
	Call Trace:
	[c0000000098c7ad0] [c00000001416e400] 0xc00000001416e400 (unreliable)
	[c0000000098c7b20] [c00000000049c2a4] wp_page_copy+0x384/0xec0
	[c0000000098c7bf0] [c0000000004a4f64] __handle_mm_fault+0x9d4/0xfb0
	[c0000000098c7cf0] [c0000000004a5630] handle_mm_fault+0xf0/0x350
	[c0000000098c7d40] [c000000000094e8c] ___do_page_fault+0x48c/0xc90
	[c0000000098c7df0] [c0000000000958a0] hash__do_page_fault+0x30/0x70
	[c0000000098c7e20] [c00000000009e244] do_hash_fault+0x1a4/0x330
	[c0000000098c7e50] [c000000000008918] 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: stable@vger.kernel.org
Fixes: baa49d81a94b ("powerpc/pseries: hvcall stack frame overhead")
Co-developed-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 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)
 plpar_hcall9_trace:
 	HCALL_INST_PRECALL(R5)
 
-	std	r4,STK_PARAM(R4)(r1)
 	mr	r0,r4
 
 	mr	r4,r5
-- 
2.39.3

[PATCH 2/2] powerpc/platforms/pseries: Remove unused r0 in the hcall tracing code

[Athira Rajeev]

In the plpar_hcall trace code, currently we use r0
to store the ORed result of r4. But this value is not
used subsequently in the code. Hence remove this unused
save to r0 in plpar_hcall and plpar_hcall9

Suggested-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/hvCall.S | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
index 2addf2ea03f0..2b0cac6fb61f 100644
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -184,8 +184,6 @@ _GLOBAL_TOC(plpar_hcall)
 plpar_hcall_trace:
 	HCALL_INST_PRECALL(R5)
 
-	mr	r0,r4
-
 	mr	r4,r5
 	mr	r5,r6
 	mr	r6,r7
@@ -295,8 +293,6 @@ _GLOBAL_TOC(plpar_hcall9)
 plpar_hcall9_trace:
 	HCALL_INST_PRECALL(R5)
 
-	mr	r0,r4
-
 	mr	r4,r5
 	mr	r5,r6
 	mr	r6,r7
-- 
2.39.3

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

[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