[REVIEW][PATCH 0/9] signal/powerpc: siginfo cleanups

21 messages, 3 authors, 2018-09-21 · open the first message on its own page

[REVIEW][PATCH 0/9] signal/powerpc: siginfo cleanups

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:41:26

This is the continuation of my work to sort out signaling of exceptions
with siginfo.  The old functions by passing siginfo resulted in many
cases of fields of siginfo that were not initialized and then passed to
userspace, and also resulted in callers getting confused and
initializing the wrong fields.  My remedy is to have specific functions
for sending each different kind of signal with siginfo.  Those functions
take the information needed to fill in siginfo and do the work
themselves.

This is my set of changes to update powerpc to use those functions.
Along with some refactoring so those functions can be cleanly used.

Folks please review and double check me.  I think I have kept these
changes simple and obviously correct but I am human and mess up
sometimes.

After these patches have had a chance to be reviewed I plan to merge
them by my siginfo tree.  If you would rather take them in the powerpc
tree let me know.   All of the prerequisites should have been merged
through Linus's tree several releases ago.

Eric W. Biederman (9):
      signal/powerpc: Use force_sig_mceerr as appropriate
      signal/powerpc: Remove pkey parameter from __bad_area
      signal/powerpc: Call _exception_pkey directly from bad_key_fault_exception
      signal/powerpc: Remove pkey parameter from __bad_area_nosemaphore
      signal/powerpc: Factor the common exception code into exception_common
      signal/powerpc: Call force_sig_fault from _exception
      signal/poewrpc: Specialize _exception_pkey for handling pkey exceptions
      signal/powerpc: Simplify _exception_pkey by using force_sig_pkuerr
      signal/powerpc: Use force_sig_fault where appropriate

 arch/powerpc/include/asm/bug.h            |  2 +-
 arch/powerpc/kernel/process.c             |  9 +----
 arch/powerpc/kernel/traps.c               | 27 ++++++++-------
 arch/powerpc/mm/fault.c                   | 55 +++++++++++++++++--------------
 arch/powerpc/platforms/cell/spu_base.c    |  4 +--
 arch/powerpc/platforms/cell/spufs/fault.c | 26 +++++----------
 6 files changed, 57 insertions(+), 66 deletions(-)

Eric

[REVIEW][PATCH 1/9] signal/powerpc: Use force_sig_mceerr as appropriate

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:09

In do_sigbus isolate the mceerr signaling code and call
force_sig_mceerr instead of falling through to the force_sig_info that
works for all of the other signals.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/mm/fault.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index d51cf5f4e45e..22d7f8748cd7 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -158,7 +158,6 @@ static int do_sigbus(struct pt_regs *regs, unsigned long address,
 		     vm_fault_t fault)
 {
 	siginfo_t info;
-	unsigned int lsb = 0;
 
 	if (!user_mode(regs))
 		return SIGBUS;
@@ -171,17 +170,22 @@ static int do_sigbus(struct pt_regs *regs, unsigned long address,
 	info.si_addr = (void __user *)address;
 #ifdef CONFIG_MEMORY_FAILURE
 	if (fault & (VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) {
+		unsigned int lsb = 0; /* shutup gcc */
+
 		pr_err("MCE: Killing %s:%d due to hardware memory corruption fault at %lx\n",
 			current->comm, current->pid, address);
-		info.si_code = BUS_MCEERR_AR;
+
+		if (fault & VM_FAULT_HWPOISON_LARGE)
+			lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
+		if (fault & VM_FAULT_HWPOISON)
+			lsb = PAGE_SHIFT;
+
+		force_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb,
+				 current);
+		return 0;
 	}
 
-	if (fault & VM_FAULT_HWPOISON_LARGE)
-		lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
-	if (fault & VM_FAULT_HWPOISON)
-		lsb = PAGE_SHIFT;
 #endif
-	info.si_addr_lsb = lsb;
 	force_sig_info(SIGBUS, &info, current);
 	return 0;
 }
-- 
2.17.1

[REVIEW][PATCH 2/9] signal/powerpc: Remove pkey parameter from __bad_area

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:27

There are no callers of __bad_area that pass in a pkey parameter so it makes
no sense to take one.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/mm/fault.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 22d7f8748cd7..e5725fa96a48 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -124,8 +124,7 @@ static noinline int bad_area_nosemaphore(struct pt_regs *regs, unsigned long add
 	return __bad_area_nosemaphore(regs, address, SEGV_MAPERR, 0);
 }
 
-static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code,
-			int pkey)
+static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code)
 {
 	struct mm_struct *mm = current->mm;
 
@@ -135,12 +134,12 @@ static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code,
 	 */
 	up_read(&mm->mmap_sem);
 
-	return __bad_area_nosemaphore(regs, address, si_code, pkey);
+	return __bad_area_nosemaphore(regs, address, si_code, 0);
 }
 
 static noinline int bad_area(struct pt_regs *regs, unsigned long address)
 {
-	return __bad_area(regs, address, SEGV_MAPERR, 0);
+	return __bad_area(regs, address, SEGV_MAPERR);
 }
 
 static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
@@ -151,7 +150,7 @@ static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
 
 static noinline int bad_access(struct pt_regs *regs, unsigned long address)
 {
-	return __bad_area(regs, address, SEGV_ACCERR, 0);
+	return __bad_area(regs, address, SEGV_ACCERR);
 }
 
 static int do_sigbus(struct pt_regs *regs, unsigned long address,
-- 
2.17.1

[REVIEW][PATCH 3/9] signal/powerpc: Call _exception_pkey directly from bad_key_fault_exception

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:33

This removes the need for other code paths to deal with pkey exceptions.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/mm/fault.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index e5725fa96a48..5afc1ee55043 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -145,7 +145,17 @@ static noinline int bad_area(struct pt_regs *regs, unsigned long address)
 static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
 				    int pkey)
 {
-	return __bad_area_nosemaphore(regs, address, SEGV_PKUERR, pkey);
+	/*
+	 * If we are in kernel mode, bail out with a SEGV, this will
+	 * be caught by the assembly which will restore the non-volatile
+	 * registers before calling bad_page_fault()
+	 */
+	if (!user_mode(regs))
+		return SIGSEGV;
+
+	_exception_pkey(SIGSEGV, regs, SEGV_PKUERR, address, pkey);
+
+	return 0;
 }
 
 static noinline int bad_access(struct pt_regs *regs, unsigned long address)
-- 
2.17.1

[REVIEW][PATCH 4/9] signal/powerpc: Remove pkey parameter from __bad_area_nosemaphore

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:38

Now that bad_key_fault_exception no longer calls __bad_area_nosemaphore
there is no reason for __bad_area_nosemaphore to handle pkeys.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/mm/fault.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 5afc1ee55043..a84d06b7d50d 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -103,8 +103,7 @@ static bool store_updates_sp(unsigned int inst)
  */
 
 static int
-__bad_area_nosemaphore(struct pt_regs *regs, unsigned long address, int si_code,
-		int pkey)
+__bad_area_nosemaphore(struct pt_regs *regs, unsigned long address, int si_code)
 {
 	/*
 	 * If we are in kernel mode, bail out with a SEGV, this will
@@ -114,14 +113,14 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long address, int si_code,
 	if (!user_mode(regs))
 		return SIGSEGV;
 
-	_exception_pkey(SIGSEGV, regs, si_code, address, pkey);
+	_exception(SIGSEGV, regs, si_code, address);
 
 	return 0;
 }
 
 static noinline int bad_area_nosemaphore(struct pt_regs *regs, unsigned long address)
 {
-	return __bad_area_nosemaphore(regs, address, SEGV_MAPERR, 0);
+	return __bad_area_nosemaphore(regs, address, SEGV_MAPERR);
 }
 
 static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code)
@@ -134,7 +133,7 @@ static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code)
 	 */
 	up_read(&mm->mmap_sem);
 
-	return __bad_area_nosemaphore(regs, address, si_code, 0);
+	return __bad_area_nosemaphore(regs, address, si_code);
 }
 
 static noinline int bad_area(struct pt_regs *regs, unsigned long address)
-- 
2.17.1

[REVIEW][PATCH 5/9] signal/powerpc: Factor the common exception code into exception_common

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:43

It is brittle and wrong to populate si_pkey when there was not a pkey
exception.  The field does not exist for all si_codes and in some
cases another field exists in the same memory location.

So factor out the code that all exceptions handlers must run
into exception_common, leaving the individual exception handlers
to generate the signals themselves.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/kernel/traps.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index f651fa91cdc9..f6c778b5144f 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -338,14 +338,12 @@ static void show_signal_msg(int signr, struct pt_regs *regs, int code,
 	show_user_instructions(regs);
 }
 
-void _exception_pkey(int signr, struct pt_regs *regs, int code,
-		     unsigned long addr, int key)
+static bool exception_common(int signr, struct pt_regs *regs, int code,
+			      unsigned long addr)
 {
-	siginfo_t info;
-
 	if (!user_mode(regs)) {
 		die("Exception in kernel mode", regs, signr);
-		return;
+		return false;
 	}
 
 	show_signal_msg(signr, regs, code, addr);
@@ -361,6 +359,16 @@ void _exception_pkey(int signr, struct pt_regs *regs, int code,
 	 */
 	thread_pkey_regs_save(&current->thread);
 
+	return true;
+}
+
+void _exception_pkey(int signr, struct pt_regs *regs, int code, unsigned long addr, int key)
+{
+	siginfo_t info;
+
+	if (!exception_common(signr, regs, code, addr))
+		return;
+
 	clear_siginfo(&info);
 	info.si_signo = signr;
 	info.si_code = code;
-- 
2.17.1

[REVIEW][PATCH 6/9] signal/powerpc: Call force_sig_fault from _exception

From: Eric W. Biederman <hidden>
Date: 2018-09-18 17:59:48

The callers of _exception don't need the pkey exception logic because
they are not processing a pkey exception.  So just call exception_common
directly and then call force_sig_fault to generate the appropriate siginfo
and deliver the appropriate signal.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/kernel/traps.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index f6c778b5144f..c38bec51dd84 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -380,7 +380,10 @@ void _exception_pkey(int signr, struct pt_regs *regs, int code, unsigned long ad
 
 void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
 {
-	_exception_pkey(signr, regs, code, addr, 0);
+	if (!exception_common(signr, regs, code, addr))
+		return;
+
+	force_sig_fault(signr, code, (void __user *)addr, current);
 }
 
 void system_reset_exception(struct pt_regs *regs)
-- 
2.17.1

[REVIEW][PATCH 7/9] signal/poewrpc: Specialize _exception_pkey for handling pkey exceptions

From: Eric W. Biederman <hidden>
Date: 2018-09-18 18:00:10

Now that _exception no longer calls _exception_pkey it is no longer
necessary to handle any signal with any si_code.  All pkey exceptions
are SIGSEGV with paired with SEGV_PKUERR.  So just handle
that case and remove the now unnecessary parameters from _exception_pkey.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/include/asm/bug.h |  2 +-
 arch/powerpc/kernel/traps.c    | 10 +++++-----
 arch/powerpc/mm/fault.c        |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index fd06dbe7d7d3..fed7e6241349 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -133,7 +133,7 @@ struct pt_regs;
 extern int do_page_fault(struct pt_regs *, unsigned long, unsigned long);
 extern void bad_page_fault(struct pt_regs *, unsigned long, int);
 extern void _exception(int, struct pt_regs *, int, unsigned long);
-extern void _exception_pkey(int, struct pt_regs *, int, unsigned long, int);
+extern void _exception_pkey(struct pt_regs *, unsigned long, int);
 extern void die(const char *, struct pt_regs *, long);
 extern bool die_will_crash(void);
 extern void panic_flush_kmsg_start(void);
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index c38bec51dd84..e5ea69222459 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -362,20 +362,20 @@ static bool exception_common(int signr, struct pt_regs *regs, int code,
 	return true;
 }
 
-void _exception_pkey(int signr, struct pt_regs *regs, int code, unsigned long addr, int key)
+void _exception_pkey(struct pt_regs *regs, unsigned long addr, int key)
 {
 	siginfo_t info;
 
-	if (!exception_common(signr, regs, code, addr))
+	if (!exception_common(SIGSEGV, regs, SEGV_PKUERR, addr))
 		return;
 
 	clear_siginfo(&info);
-	info.si_signo = signr;
-	info.si_code = code;
+	info.si_signo = SIGSEGV;
+	info.si_code = SEGV_PKUERR;
 	info.si_addr = (void __user *) addr;
 	info.si_pkey = key;
 
-	force_sig_info(signr, &info, current);
+	force_sig_info(info.si_signo, &info, current);
 }
 
 void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index a84d06b7d50d..406d0e0ef096 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -152,7 +152,7 @@ static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
 	if (!user_mode(regs))
 		return SIGSEGV;
 
-	_exception_pkey(SIGSEGV, regs, SEGV_PKUERR, address, pkey);
+	_exception_pkey(regs, address, pkey);
 
 	return 0;
 }
-- 
2.17.1

[REVIEW][PATCH 8/9] signal/powerpc: Simplify _exception_pkey by using force_sig_pkuerr

From: Eric W. Biederman <hidden>
Date: 2018-09-18 18:00:13

Call force_sig_pkuerr directly instead of rolling it by hand
in _exception_pkey.

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/kernel/traps.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index e5ea69222459..ab1bd06d7c44 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -364,18 +364,10 @@ static bool exception_common(int signr, struct pt_regs *regs, int code,
 
 void _exception_pkey(struct pt_regs *regs, unsigned long addr, int key)
 {
-	siginfo_t info;
-
 	if (!exception_common(SIGSEGV, regs, SEGV_PKUERR, addr))
 		return;
 
-	clear_siginfo(&info);
-	info.si_signo = SIGSEGV;
-	info.si_code = SEGV_PKUERR;
-	info.si_addr = (void __user *) addr;
-	info.si_pkey = key;
-
-	force_sig_info(info.si_signo, &info, current);
+	force_sig_pkuerr((void __user *) addr, key);
 }
 
 void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
-- 
2.17.1

[REVIEW][PATCH 9/9] signal/powerpc: Use force_sig_fault where appropriate

From: Eric W. Biederman <hidden>
Date: 2018-09-18 18:00:32

Signed-off-by: "Eric W. Biederman" <redacted>
---
 arch/powerpc/kernel/process.c             |  9 +-------
 arch/powerpc/mm/fault.c                   |  9 +-------
 arch/powerpc/platforms/cell/spu_base.c    |  4 ++--
 arch/powerpc/platforms/cell/spufs/fault.c | 26 +++++++----------------
 4 files changed, 12 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 913c5725cdb2..553a396e7fc1 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -620,8 +620,6 @@ void do_send_trap(struct pt_regs *regs, unsigned long address,
 void do_break (struct pt_regs *regs, unsigned long address,
 		    unsigned long error_code)
 {
-	siginfo_t info;
-
 	current->thread.trap_nr = TRAP_HWBKPT;
 	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
 			11, SIGSEGV) == NOTIFY_STOP)
@@ -634,12 +632,7 @@ void do_break (struct pt_regs *regs, unsigned long address,
 	hw_breakpoint_disable();
 
 	/* Deliver the signal to userspace */
-	clear_siginfo(&info);
-	info.si_signo = SIGTRAP;
-	info.si_errno = 0;
-	info.si_code = TRAP_HWBKPT;
-	info.si_addr = (void __user *)address;
-	force_sig_info(SIGTRAP, &info, current);
+	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)address, current);
 }
 #endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
 
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 406d0e0ef096..1697e903bbf2 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -165,17 +165,10 @@ static noinline int bad_access(struct pt_regs *regs, unsigned long address)
 static int do_sigbus(struct pt_regs *regs, unsigned long address,
 		     vm_fault_t fault)
 {
-	siginfo_t info;
-
 	if (!user_mode(regs))
 		return SIGBUS;
 
 	current->thread.trap_nr = BUS_ADRERR;
-	clear_siginfo(&info);
-	info.si_signo = SIGBUS;
-	info.si_errno = 0;
-	info.si_code = BUS_ADRERR;
-	info.si_addr = (void __user *)address;
 #ifdef CONFIG_MEMORY_FAILURE
 	if (fault & (VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) {
 		unsigned int lsb = 0; /* shutup gcc */
@@ -194,7 +187,7 @@ static int do_sigbus(struct pt_regs *regs, unsigned long address,
 	}
 
 #endif
-	force_sig_info(SIGBUS, &info, current);
+	force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, current);
 	return 0;
 }
 
diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
index 0c45cdbac4cf..7f12c7b78c0f 100644
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -50,11 +50,11 @@ struct cbe_spu_info cbe_spu_info[MAX_NUMNODES];
 EXPORT_SYMBOL_GPL(cbe_spu_info);
 
 /*
- * The spufs fault-handling code needs to call force_sig_info to raise signals
+ * The spufs fault-handling code needs to call force_sig_fault to raise signals
  * on DMA errors. Export it here to avoid general kernel-wide access to this
  * function
  */
-EXPORT_SYMBOL_GPL(force_sig_info);
+EXPORT_SYMBOL_GPL(force_sig_fault);
 
 /*
  * Protects cbe_spu_info and spu->number.
diff --git a/arch/powerpc/platforms/cell/spufs/fault.c b/arch/powerpc/platforms/cell/spufs/fault.c
index 83cf58daaa79..971ac43b5d60 100644
--- a/arch/powerpc/platforms/cell/spufs/fault.c
+++ b/arch/powerpc/platforms/cell/spufs/fault.c
@@ -36,42 +36,32 @@
 static void spufs_handle_event(struct spu_context *ctx,
 				unsigned long ea, int type)
 {
-	siginfo_t info;
-
 	if (ctx->flags & SPU_CREATE_EVENTS_ENABLED) {
 		ctx->event_return |= type;
 		wake_up_all(&ctx->stop_wq);
 		return;
 	}
 
-	clear_siginfo(&info);
-
 	switch (type) {
 	case SPE_EVENT_INVALID_DMA:
-		info.si_signo = SIGBUS;
-		info.si_code = BUS_OBJERR;
+		force_sig_fault(SIGBUS, BUS_OBJERR, NULL, current);
 		break;
 	case SPE_EVENT_SPE_DATA_STORAGE:
-		info.si_signo = SIGSEGV;
-		info.si_addr = (void __user *)ea;
-		info.si_code = SEGV_ACCERR;
 		ctx->ops->restart_dma(ctx);
+		force_sig_fault(SIGSEGV, SEGV_ACCERR, (void __user *)ea,
+				current);
 		break;
 	case SPE_EVENT_DMA_ALIGNMENT:
-		info.si_signo = SIGBUS;
 		/* DAR isn't set for an alignment fault :( */
-		info.si_code = BUS_ADRALN;
+		force_sig_fault(SIGBUS, BUS_ADRALN, NULL, current);
 		break;
 	case SPE_EVENT_SPE_ERROR:
-		info.si_signo = SIGILL;
-		info.si_addr = (void __user *)(unsigned long)
-			ctx->ops->npc_read(ctx) - 4;
-		info.si_code = ILL_ILLOPC;
+		force_sig_fault(
+			SIGILL, ILL_ILLOPC,
+			(void __user *)(unsigned long)
+			ctx->ops->npc_read(ctx) - 4, current);
 		break;
 	}
-
-	if (info.si_signo)
-		force_sig_info(info.si_signo, &info, current);
 }
 
 int spufs_handle_class0(struct spu_context *ctx)
-- 
2.17.1

Re: [REVIEW][PATCH 1/9] signal/powerpc: Use force_sig_mceerr as appropriate

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:21:06

Hi Eric,

On Tue, 18 Sep 2018 19:58:42 +0200 "Eric W. Biederman" [off-list ref] wrote:
In do_sigbus isolate the mceerr signaling code and call
force_sig_mceerr instead of falling through to the force_sig_info that
works for all of the other signals.

Signed-off-by: "Eric W. Biederman" <redacted>
Looks good to me.  I was going to mention further cleanup, but I see
you do that in a later patch.

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 2/9] signal/powerpc: Remove pkey parameter from __bad_area

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:23:42

Hi Eric,

On Tue, 18 Sep 2018 19:58:43 +0200 "Eric W. Biederman" [off-list ref] wrote:
There are no callers of __bad_area that pass in a pkey parameter so it makes
no sense to take one.

Signed-off-by: "Eric W. Biederman" <redacted>
Fairly straight forward.

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 3/9] signal/powerpc: Call _exception_pkey directly from bad_key_fault_exception

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:27:26

Hi Eric,

On Tue, 18 Sep 2018 19:58:44 +0200 "Eric W. Biederman" [off-list ref] wrote:
This removes the need for other code paths to deal with pkey exceptions.

Signed-off-by: "Eric W. Biederman" <redacted>
Straight forward expansion.

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 4/9] signal/powerpc: Remove pkey parameter from __bad_area_nosemaphore

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:32:33

Hi Eric,

On Tue, 18 Sep 2018 19:58:45 +0200 "Eric W. Biederman" [off-list ref] wrote:
Now that bad_key_fault_exception no longer calls __bad_area_nosemaphore
there is no reason for __bad_area_nosemaphore to handle pkeys.

Signed-off-by: "Eric W. Biederman" <redacted>
Again, very straight forward given that _exception(a,b,c,d) -> _exception_pkey(a,b,c,d,0).

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 5/9] signal/powerpc: Factor the common exception code into exception_common

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:38:14

Hi Eric,

On Tue, 18 Sep 2018 19:58:46 +0200 "Eric W. Biederman" [off-list ref] wrote:
It is brittle and wrong to populate si_pkey when there was not a pkey
exception.  The field does not exist for all si_codes and in some
cases another field exists in the same memory location.

So factor out the code that all exceptions handlers must run
into exception_common, leaving the individual exception handlers
to generate the signals themselves.

Signed-off-by: "Eric W. Biederman" <redacted>
Clearly no change in semantics.

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 6/9] signal/powerpc: Call force_sig_fault from _exception

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:48:24

Hi Eric,

On Tue, 18 Sep 2018 19:58:47 +0200 "Eric W. Biederman" [off-list ref] wrote:
The callers of _exception don't need the pkey exception logic because
they are not processing a pkey exception.  So just call exception_common
directly and then call force_sig_fault to generate the appropriate siginfo
and deliver the appropriate signal.

Signed-off-by: "Eric W. Biederman" <redacted>
Looks right to me.

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 7/9] signal/poewrpc: Specialize _exception_pkey for handling pkey exceptions

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:54:35

Hi Eric,

On Tue, 18 Sep 2018 19:58:48 +0200 "Eric W. Biederman" [off-list ref] wrote:
Now that _exception no longer calls _exception_pkey it is no longer
necessary to handle any signal with any si_code.  All pkey exceptions
are SIGSEGV with paired with SEGV_PKUERR.  So just handle
that case and remove the now unnecessary parameters from _exception_pkey.

Signed-off-by: "Eric W. Biederman" <redacted>
Looks fine to me (small query below).

Reviewed-by: Stephen Rothwell <redacted>
 	clear_siginfo(&info);
-	info.si_signo = signr;
-	info.si_code = code;
+	info.si_signo = SIGSEGV;
+	info.si_code = SEGV_PKUERR;
 	info.si_addr = (void __user *) addr;
 	info.si_pkey = key;
 
-	force_sig_info(signr, &info, current);
+	force_sig_info(info.si_signo, &info, current);
                       ^^^^^^^^^^^^^
Why not just SIGSEGV?

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 8/9] signal/powerpc: Simplify _exception_pkey by using force_sig_pkuerr

From: Stephen Rothwell <hidden>
Date: 2018-09-21 08:57:49

Hi Eric,

On Tue, 18 Sep 2018 19:58:49 +0200 "Eric W. Biederman" [off-list ref] wrote:
Call force_sig_pkuerr directly instead of rolling it by hand
in _exception_pkey.

Signed-off-by: "Eric W. Biederman" <redacted>
You can ignore the question in the previous email :-)

Reviewed-by: Stephen Rothwell <redacted>

-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 7/9] signal/poewrpc: Specialize _exception_pkey for handling pkey exceptions

From: Christophe LEROY <hidden>
Date: 2018-09-21 08:58:50


Le 21/09/2018 à 10:54, Stephen Rothwell a écrit :
Hi Eric,

On Tue, 18 Sep 2018 19:58:48 +0200 "Eric W. Biederman" [off-list ref] wrote:
quoted
Now that _exception no longer calls _exception_pkey it is no longer
necessary to handle any signal with any si_code.  All pkey exceptions
are SIGSEGV with paired with SEGV_PKUERR.  So just handle
that case and remove the now unnecessary parameters from _exception_pkey.

Signed-off-by: "Eric W. Biederman" <redacted>
Looks fine to me (small query below).

Reviewed-by: Stephen Rothwell <redacted>
In the patch title, s/poewrpc/powerpc
quoted
  	clear_siginfo(&info);
-	info.si_signo = signr;
-	info.si_code = code;
+	info.si_signo = SIGSEGV;
+	info.si_code = SEGV_PKUERR;
  	info.si_addr = (void __user *) addr;
  	info.si_pkey = key;
  
-	force_sig_info(signr, &info, current);
+	force_sig_info(info.si_signo, &info, current);
                        ^^^^^^^^^^^^^
Why not just SIGSEGV?

Re: [REVIEW][PATCH 9/9] signal/powerpc: Use force_sig_fault where appropriate

From: Stephen Rothwell <hidden>
Date: 2018-09-21 09:05:38

Hi Eric,

On Tue, 18 Sep 2018 19:58:50 +0200 "Eric W. Biederman" [off-list ref] wrote:
Signed-off-by: "Eric W. Biederman" <redacted>
Again, looks all good to me.

Reviewed-by: Stephen Rothwell <redacted>
-- 
Cheers,
Stephen Rothwell

Re: [REVIEW][PATCH 0/9] signal/powerpc: siginfo cleanups

From: Stephen Rothwell <hidden>
Date: 2018-09-21 09:06:45

Hi Eric,

On Tue, 18 Sep 2018 19:41:09 +0200 ebiederm@xmission.com (Eric W. Biederman) wrote:
Folks please review and double check me.  I think I have kept these
changes simple and obviously correct but I am human and mess up
sometimes.
Its always good to see a nicely set out patch series like this.

-- 
Cheers,
Stephen Rothwell
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help