Thread (10 messages) flat view 10 messages, 4 authors, 2018-10-16

Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs

From: Madhavan Srinivasan <hidden>
Date: 2018-10-15 12:43:18


On Monday 15 October 2018 04:38 PM, Michael Ellerman wrote:
Madhavan Srinivasan [off-list ref] writes:
quoted
On Saturday 13 October 2018 04:26 PM, Michael Ellerman wrote:
quoted
We use a shared definition for struct pt_regs in uapi/asm/ptrace.h.
That means the layout of the structure is ABI, ie. we can't change it.

That would be fine if it was only used to describe the user-visible
register state of a process, but it's also the struct we use in the
kernel to describe the registers saved in an interrupt frame.

We'd like more flexibility in the content (and possibly layout) of the
kernel version of the struct, but currently that's not possible.

So split the definition into a user-visible definition which remains
unchanged, and a kernel internal one.

At the moment they're still identical, and we check that at build
time. That's because we have code (in ptrace etc.) that assumes that
they are the same. We will fix that code in future patches, and then
we can break the strict symmetry between the two structs.
Nice and awesome. But just trying to understand. What will
*regs will point to in the "struct sigcontext".
Yeah that's a bit fishy.

It should always point to a user_pt_regs.

So in the kernel we want:

   struct sigcontext {
   	...
   	struct user_pt_regs	__user *regs;

And in userspace we want:

   struct sigcontext {
   	...
   	struct pt_regs	__user *regs;


I think it's not actually broken at the moment, because it's just a
pointer, and we don't do anything based on the sizeof() the type.
yes. This clarifies. But still perf/perf_regs.c needs changes.
Because perf support dumping user_space regs and interrupt regs.
Once again, we dont use any sizeof(), but need to handle the
user_pt_regs changes.

I will have a look at that in the morning.

Thanks for clarification.
Maddy
quoted hunk ↗ jump to hunk
But still we should fix it.

I guess I'll do this:
diff --git a/arch/powerpc/include/uapi/asm/sigcontext.h b/arch/powerpc/include/uapi/asm/sigcontext.h
index 2fbe485acdb4..630aeda56d59 100644
--- a/arch/powerpc/include/uapi/asm/sigcontext.h
+++ b/arch/powerpc/include/uapi/asm/sigcontext.h
@@ -22,7 +22,11 @@ struct sigcontext {
  #endif
  	unsigned long	handler;
  	unsigned long	oldmask;
-	struct pt_regs	__user *regs;
+#ifdef __KERNEL__
+	struct user_pt_regs __user *regs;
+#else
+	struct pt_regs	*regs;
+#endif
  #ifdef __powerpc64__
  	elf_gregset_t	gp_regs;
  	elf_fpregset_t	fp_regs;

Thanks for the review.

cheers
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help