[REVIEW][PATCH 0/3] signal/unicore32: siginfo cleanups

[REVIEW][PATCH 0/3] signal/unicore32: siginfo cleanups

[Eric W. Biederman]

I have been slowly cleaning up the architectues ever since I discovered
that the pattern of passing in struct siginfo is error prone, and
occassionally results in broken siginfo being sent to userspace.

I don't have a clue how to obtain a working unicore32 compiler so the
following changes have only been reviewed carefully and not compile
tested.  Still I believe they are simple and obviously correct.

Anyone who can review or test these changes please do and let me know
about any problems you find so that I can correct them.

My intention is to merge this through my siginfo tree.  If you feel it
should go through your arch tree let me know.   All of the prerequisites
should have been merged several releases ago.

Eric W. Biederman (3):
      signal/unicore32: Use send_sig_fault where appropriate
      signal/unicore32: Generate siginfo in ucs32_notify_die
      signal/unicore32: Use force_sig_fault where appropriate

 arch/unicore32/include/asm/bug.h  |  3 ++-
 arch/unicore32/kernel/fpu-ucf64.c | 12 +++---------
 arch/unicore32/kernel/traps.c     |  5 +++--
 arch/unicore32/mm/fault.c         | 27 +++++----------------------
 4 files changed, 13 insertions(+), 34 deletions(-)

Eric

[REVIEW][PATCH 0/3] signal/unicore32: siginfo cleanups

[Eric W. Biederman]

I have been slowly cleaning up the architectues ever since I discovered
that the pattern of passing in struct siginfo is error prone, and
occassionally results in broken siginfo being sent to userspace.

I don't have a clue how to obtain a working unicore32 compiler so the
following changes have only been reviewed carefully and not compile
tested.  Still I believe they are simple and obviously correct.

Anyone who can review or test these changes please do and let me know
about any problems you find so that I can correct them.

My intention is to merge this through my siginfo tree.  If you feel it
should go through your arch tree let me know.   All of the prerequisites
should have been merged several releases ago.

Eric W. Biederman (3):
      signal/unicore32: Use send_sig_fault where appropriate
      signal/unicore32: Generate siginfo in ucs32_notify_die
      signal/unicore32: Use force_sig_fault where appropriate

 arch/unicore32/include/asm/bug.h  |  3 ++-
 arch/unicore32/kernel/fpu-ucf64.c | 12 +++---------
 arch/unicore32/kernel/traps.c     |  5 +++--
 arch/unicore32/mm/fault.c         | 27 +++++----------------------
 4 files changed, 13 insertions(+), 34 deletions(-)

Eric

[REVIEW][PATCH 1/3] signal/unicore32: Use send_sig_fault where appropriate

[Eric W. Biederman]

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/unicore32/kernel/fpu-ucf64.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/unicore32/kernel/fpu-ucf64.c b/arch/unicore32/kernel/fpu-ucf64.c
index 8594b168f25e..fc5dad32a982 100644
--- a/arch/unicore32/kernel/fpu-ucf64.c
+++ b/arch/unicore32/kernel/fpu-ucf64.c
@@ -54,14 +54,6 @@
  */
 void ucf64_raise_sigfpe(struct pt_regs *regs)
 {
-	siginfo_t info;
-
-	clear_siginfo(&info);
-
-	info.si_signo = SIGFPE;
-	info.si_code = FPE_FLTUNK;
-	info.si_addr = (void __user *)(instruction_pointer(regs) - 4);
-
 	/*
 	 * This is the same as NWFPE, because it's not clear what
 	 * this is used for
@@ -69,7 +61,9 @@ void ucf64_raise_sigfpe(struct pt_regs *regs)
 	current->thread.error_code = 0;
 	current->thread.trap_no = 6;
 
-	send_sig_info(SIGFPE, &info, current);
+	send_sig_fault(SIGFPE, FPE_FLTUNK,
+		       (void __user *)(instruction_pointer(regs) - 4),
+		       current);
 }
 
 /*
-- 
2.17.1

[REVIEW][PATCH 2/3] signal/unicore32: Generate siginfo in ucs32_notify_die

[Eric W. Biederman]

Pass the signal number, and the signal code, and the faulting
address into uc32_notify_die so the callers do not need
to generate a struct siginfo.

In ucs32_ntoify_die use the newly passed in information to
call force_sig_fault to generate the siginfo and send the error.

This simplifies the code making the chances of bugs much less likely.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/unicore32/include/asm/bug.h |  3 ++-
 arch/unicore32/kernel/traps.c    |  5 +++--
 arch/unicore32/mm/fault.c        | 18 ++++--------------
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/arch/unicore32/include/asm/bug.h b/arch/unicore32/include/asm/bug.h
index 93a56f3e2344..83c7687a0e61 100644
--- a/arch/unicore32/include/asm/bug.h
+++ b/arch/unicore32/include/asm/bug.h
@@ -17,6 +17,7 @@ struct siginfo;
 
 extern void die(const char *msg, struct pt_regs *regs, int err);
 extern void uc32_notify_die(const char *str, struct pt_regs *regs,
-		struct siginfo *info, unsigned long err, unsigned long trap);
+		int sig, int code, void __user *addr,
+		unsigned long err, unsigned long trap);
 
 #endif /* __UNICORE_BUG_H__ */
diff --git a/arch/unicore32/kernel/traps.c b/arch/unicore32/kernel/traps.c
index c4ac6043ebb0..fb376d83e043 100644
--- a/arch/unicore32/kernel/traps.c
+++ b/arch/unicore32/kernel/traps.c
@@ -241,13 +241,14 @@ void die(const char *str, struct pt_regs *regs, int err)
 }
 
 void uc32_notify_die(const char *str, struct pt_regs *regs,
-		struct siginfo *info, unsigned long err, unsigned long trap)
+		int sig, int code, void __user *addr,
+		unsigned long err, unsigned long trap)
 {
 	if (user_mode(regs)) {
 		current->thread.error_code = err;
 		current->thread.trap_no = trap;
 
-		force_sig_info(info->si_signo, info, current);
+		force_sig_fault(sig, code, addr, current);
 	} else
 		die(str, regs, err);
 }
diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index 8f12a5b50a42..a942776110a0 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -466,7 +466,6 @@ asmlinkage void do_DataAbort(unsigned long addr, unsigned int fsr,
 			struct pt_regs *regs)
 {
 	const struct fsr_info *inf = fsr_info + fsr_fs(fsr);
-	struct siginfo info;
 
 	if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
 		return;
@@ -474,19 +473,14 @@ asmlinkage void do_DataAbort(unsigned long addr, unsigned int fsr,
 	printk(KERN_ALERT "Unhandled fault: %s (0x%03x) at 0x%08lx\n",
 	       inf->name, fsr, addr);
 
-	clear_siginfo(&info);
-	info.si_signo = inf->sig;
-	info.si_errno = 0;
-	info.si_code = inf->code;
-	info.si_addr = (void __user *)addr;
-	uc32_notify_die("", regs, &info, fsr, 0);
+	uc32_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
+			fsr, 0);
 }
 
 asmlinkage void do_PrefetchAbort(unsigned long addr,
 			unsigned int ifsr, struct pt_regs *regs)
 {
 	const struct fsr_info *inf = fsr_info + fsr_fs(ifsr);
-	struct siginfo info;
 
 	if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs))
 		return;
@@ -494,10 +488,6 @@ asmlinkage void do_PrefetchAbort(unsigned long addr,
 	printk(KERN_ALERT "Unhandled prefetch abort: %s (0x%03x) at 0x%08lx\n",
 	       inf->name, ifsr, addr);
 
-	clear_siginfo(&info);
-	info.si_signo = inf->sig;
-	info.si_errno = 0;
-	info.si_code = inf->code;
-	info.si_addr = (void __user *)addr;
-	uc32_notify_die("", regs, &info, ifsr, 0);
+	uc32_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
+			ifsr, 0);
 }
-- 
2.17.1

[REVIEW][PATCH 3/3] signal/unicore32: Use force_sig_fault where appropriate

[Eric W. Biederman]

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/unicore32/mm/fault.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index a942776110a0..b9a3a50644c1 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -120,17 +120,10 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
 		unsigned int fsr, unsigned int sig, int code,
 		struct pt_regs *regs)
 {
-	struct siginfo si;
-
 	tsk->thread.address = addr;
 	tsk->thread.error_code = fsr;
 	tsk->thread.trap_no = 14;
-	clear_siginfo(&si);
-	si.si_signo = sig;
-	si.si_errno = 0;
-	si.si_code = code;
-	si.si_addr = (void __user *)addr;
-	force_sig_info(sig, &si, tsk);
+	force_sig_fault(sig, code, (void __user *)addr, tsk);
 }
 
 void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
-- 
2.17.1

Re: [REVIEW][PATCH 0/3] signal/unicore32: siginfo cleanups

[Guan Xuetao]

Hi Eric,

I'll review and check the patches after chinese national day vocation.

Thanks.

Guan Xuetao

> -----Original Messages-----
> From: "Eric W. Biederman" <ebiederm@xmission.com>
> Sent Time: 2018-09-24 23:23:35 (Monday)
> To: linux-kernel@vger.kernel.org
> Cc: linux-arch@vger.kernel.org, "Guan Xuetao" <gxt@pku.edu.cn>
> Subject: [REVIEW][PATCH 0/3] signal/unicore32: siginfo cleanups
> 
> 
> I have been slowly cleaning up the architectues ever since I discovered
> that the pattern of passing in struct siginfo is error prone, and
> occassionally results in broken siginfo being sent to userspace.
> 
> I don't have a clue how to obtain a working unicore32 compiler so the
> following changes have only been reviewed carefully and not compile
> tested.  Still I believe they are simple and obviously correct.
> 
> Anyone who can review or test these changes please do and let me know
> about any problems you find so that I can correct them.
> 
> My intention is to merge this through my siginfo tree.  If you feel it
> should go through your arch tree let me know.   All of the prerequisites
> should have been merged several releases ago.
> 
> Eric W. Biederman (3):
>       signal/unicore32: Use send_sig_fault where appropriate
>       signal/unicore32: Generate siginfo in ucs32_notify_die
>       signal/unicore32: Use force_sig_fault where appropriate
> 
>  arch/unicore32/include/asm/bug.h  |  3 ++-
>  arch/unicore32/kernel/fpu-ucf64.c | 12 +++---------
>  arch/unicore32/kernel/traps.c     |  5 +++--
>  arch/unicore32/mm/fault.c         | 27 +++++----------------------
>  4 files changed, 13 insertions(+), 34 deletions(-)
> 
> Eric