* [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate [not found] <87604mhrnb.fsf@xmission.com> @ 2018-04-20 14:37 ` Eric W. Biederman [not found] ` <e0a5bde2-7817-3e05-1c8d-c8fa8f6aa5f2@mips.com> 0 siblings, 1 reply; 5+ messages in thread From: Eric W. Biederman @ 2018-04-20 14:37 UTC (permalink / raw) To: linux-arch Cc: linux-kernel, Eric W. Biederman, Ralf Baechle, James Hogan, linux-mips Filling in struct siginfo before calling force_sig_info a tedious and error prone process, where once in a great while the wrong fields are filled out, and siginfo has been inconsistently cleared. Simplify this process by using the helper force_sig_fault. Which takes as a parameters all of the information it needs, ensures all of the fiddly bits of filling in struct siginfo are done properly and then calls force_sig_info. In short about a 5 line reduction in code for every time force_sig_info is called, which makes the calling function clearer. Cc: Ralf Baechle <ralf@linux-mips.org> Cc: James Hogan <jhogan@kernel.org> Cc: linux-mips@linux-mips.org Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- arch/mips/mm/fault.c | 19 ++++---------- 2 files changed, 23 insertions(+), 61 deletions(-) diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c index 967e9e4e795e..66ec4b0b484d 100644 --- a/arch/mips/kernel/traps.c +++ b/arch/mips/kernel/traps.c @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) asmlinkage void do_ov(struct pt_regs *regs) { enum ctx_state prev_state; - siginfo_t info; - - clear_siginfo(&info); - info.si_signo = SIGFPE; - info.si_code = FPE_INTOVF; - info.si_addr = (void __user *)regs->cp0_epc; prev_state = exception_enter(); die_if_kernel("Integer overflow", regs); - force_sig_info(SIGFPE, &info, current); + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); exception_exit(prev_state); } @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, struct task_struct *tsk) { - struct siginfo si; - - clear_siginfo(&si); - si.si_addr = fault_addr; - si.si_signo = SIGFPE; + int si_code; if (fcr31 & FPU_CSR_INV_X) - si.si_code = FPE_FLTINV; + si_code = FPE_FLTINV; else if (fcr31 & FPU_CSR_DIV_X) - si.si_code = FPE_FLTDIV; + si_code = FPE_FLTDIV; else if (fcr31 & FPU_CSR_OVF_X) - si.si_code = FPE_FLTOVF; + si_code = FPE_FLTOVF; else if (fcr31 & FPU_CSR_UDF_X) - si.si_code = FPE_FLTUND; + si_code = FPE_FLTUND; else if (fcr31 & FPU_CSR_INE_X) - si.si_code = FPE_FLTRES; + si_code = FPE_FLTRES; - force_sig_info(SIGFPE, &si, tsk); + force_sig_fault(SIGFPE, si_code, fault_addr, tsk); } int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31) { - struct siginfo si; + int si_code; struct vm_area_struct *vma; - clear_siginfo(&si); switch (sig) { case 0: return 0; @@ -757,23 +746,18 @@ int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31) return 1; case SIGBUS: - si.si_addr = fault_addr; - si.si_signo = sig; - si.si_code = BUS_ADRERR; - force_sig_info(sig, &si, current); + force_sig_fault(SIGBUS, BUS_ADRERR, fault_addr, current); return 1; case SIGSEGV: - si.si_addr = fault_addr; - si.si_signo = sig; down_read(¤t->mm->mmap_sem); vma = find_vma(current->mm, (unsigned long)fault_addr); if (vma && (vma->vm_start <= (unsigned long)fault_addr)) - si.si_code = SEGV_ACCERR; + si_code = SEGV_ACCERR; else - si.si_code = SEGV_MAPERR; + si_code = SEGV_MAPERR; up_read(¤t->mm->mmap_sem); - force_sig_info(sig, &si, current); + force_sig_fault(SIGSEGV, si_code, fault_addr, current); return 1; default: @@ -896,10 +880,8 @@ asmlinkage void do_fpe(struct pt_regs *regs, unsigned long fcr31) void do_trap_or_bp(struct pt_regs *regs, unsigned int code, int si_code, const char *str) { - siginfo_t info; char b[40]; - clear_siginfo(&info); #ifdef CONFIG_KGDB_LOW_LEVEL_TRAP if (kgdb_ll_trap(DIE_TRAP, str, regs, code, current->thread.trap_nr, SIGTRAP) == NOTIFY_STOP) @@ -921,13 +903,9 @@ void do_trap_or_bp(struct pt_regs *regs, unsigned int code, int si_code, case BRK_DIVZERO: scnprintf(b, sizeof(b), "%s instruction in kernel code", str); die_if_kernel(b, regs); - if (code == BRK_DIVZERO) - info.si_code = FPE_INTDIV; - else - info.si_code = FPE_INTOVF; - info.si_signo = SIGFPE; - info.si_addr = (void __user *) regs->cp0_epc; - force_sig_info(SIGFPE, &info, current); + force_sig_fault(SIGFPE, + code == BRK_DIVZERO ? FPE_INTDIV : FPE_INTOVF, + (void __user *) regs->cp0_epc, current); break; case BRK_BUG: die_if_kernel("Kernel bug detected", regs); @@ -952,9 +930,7 @@ void do_trap_or_bp(struct pt_regs *regs, unsigned int code, int si_code, scnprintf(b, sizeof(b), "%s instruction in kernel code", str); die_if_kernel(b, regs); if (si_code) { - info.si_signo = SIGTRAP; - info.si_code = si_code; - force_sig_info(SIGTRAP, &info, current); + force_sig_fault(SIGTRAP, si_code, NULL, current); } else { force_sig(SIGTRAP, current); } @@ -1506,13 +1482,8 @@ asmlinkage void do_mdmx(struct pt_regs *regs) */ asmlinkage void do_watch(struct pt_regs *regs) { - siginfo_t info; enum ctx_state prev_state; - clear_siginfo(&info); - info.si_signo = SIGTRAP; - info.si_code = TRAP_HWBKPT; - prev_state = exception_enter(); /* * Clear WP (bit 22) bit of cause register so we don't loop @@ -1528,7 +1499,7 @@ asmlinkage void do_watch(struct pt_regs *regs) if (test_tsk_thread_flag(current, TIF_LOAD_WATCH)) { mips_read_watch_registers(); local_irq_enable(); - force_sig_info(SIGTRAP, &info, current); + force_sig_fault(SIGTRAP, TRAP_HWBKPT, NULL, current); } else { mips_clear_watch_registers(); local_irq_enable(); diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c index 75392becd933..5f71f2b903b7 100644 --- a/arch/mips/mm/fault.c +++ b/arch/mips/mm/fault.c @@ -42,7 +42,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, struct task_struct *tsk = current; struct mm_struct *mm = tsk->mm; const int field = sizeof(unsigned long) * 2; - siginfo_t info; + int si_code; int fault; unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; @@ -63,8 +63,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, return; #endif - clear_siginfo(&info); - info.si_code = SEGV_MAPERR; + si_code = SEGV_MAPERR; /* * We fault-in kernel-space virtual memory on-demand. The @@ -113,7 +112,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, * we can handle it.. */ good_area: - info.si_code = SEGV_ACCERR; + si_code = SEGV_ACCERR; if (write) { if (!(vma->vm_flags & VM_WRITE)) @@ -224,11 +223,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, pr_cont("\n"); } current->thread.trap_nr = (regs->cp0_cause >> 2) & 0x1f; - info.si_signo = SIGSEGV; - info.si_errno = 0; - /* info.si_code has been set above */ - info.si_addr = (void __user *) address; - force_sig_info(SIGSEGV, &info, tsk); + force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk); return; } @@ -284,11 +279,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, #endif current->thread.trap_nr = (regs->cp0_cause >> 2) & 0x1f; tsk->thread.cp0_badvaddr = address; - info.si_signo = SIGBUS; - info.si_errno = 0; - info.si_code = BUS_ADRERR; - info.si_addr = (void __user *) address; - force_sig_info(SIGBUS, &info, tsk); + force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk); return; #ifndef CONFIG_64BIT -- 2.14.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <e0a5bde2-7817-3e05-1c8d-c8fa8f6aa5f2@mips.com>]
* Re: [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate [not found] ` <e0a5bde2-7817-3e05-1c8d-c8fa8f6aa5f2@mips.com> @ 2018-05-10 2:39 ` Eric W. Biederman 2018-05-10 2:39 ` Eric W. Biederman [not found] ` <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com> 0 siblings, 2 replies; 5+ messages in thread From: Eric W. Biederman @ 2018-05-10 2:39 UTC (permalink / raw) To: Matt Redfearn Cc: linux-arch, linux-kernel, Ralf Baechle, James Hogan, linux-mips Matt Redfearn <matt.redfearn@mips.com> writes: > Hi Eric, > > On 20/04/18 15:37, Eric W. Biederman wrote: >> Filling in struct siginfo before calling force_sig_info a tedious and >> error prone process, where once in a great while the wrong fields >> are filled out, and siginfo has been inconsistently cleared. >> >> Simplify this process by using the helper force_sig_fault. Which >> takes as a parameters all of the information it needs, ensures >> all of the fiddly bits of filling in struct siginfo are done properly >> and then calls force_sig_info. >> >> In short about a 5 line reduction in code for every time force_sig_info >> is called, which makes the calling function clearer. >> >> Cc: Ralf Baechle <ralf@linux-mips.org> >> Cc: James Hogan <jhogan@kernel.org> >> Cc: linux-mips@linux-mips.org >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >> --- >> arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- >> arch/mips/mm/fault.c | 19 ++++---------- >> 2 files changed, 23 insertions(+), 61 deletions(-) >> >> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c >> index 967e9e4e795e..66ec4b0b484d 100644 >> --- a/arch/mips/kernel/traps.c >> +++ b/arch/mips/kernel/traps.c >> @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) >> asmlinkage void do_ov(struct pt_regs *regs) >> { >> enum ctx_state prev_state; >> - siginfo_t info; >> - >> - clear_siginfo(&info); >> - info.si_signo = SIGFPE; >> - info.si_code = FPE_INTOVF; >> - info.si_addr = (void __user *)regs->cp0_epc; >> prev_state = exception_enter(); >> die_if_kernel("Integer overflow", regs); >> - force_sig_info(SIGFPE, &info, current); >> + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); >> exception_exit(prev_state); >> } >> @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) >> void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, >> struct task_struct *tsk) >> { >> - struct siginfo si; >> - >> - clear_siginfo(&si); >> - si.si_addr = fault_addr; >> - si.si_signo = SIGFPE; >> + int si_code; > > This is giving build errors in Linux next > (https://storage.kernelci.org/next/master/next-20180509/mips/defconfig+kselftest/build.log) > > si_code would have ended up as 0 before from the clear_siginfo(), but perhaps And si_code 0 is not a valid si_code to use with a floating point siginfo layout. > int si_code = FPE_FLTUNK; > > Would make a more sensible default? FPE_FLTUNK would make a more sensible default. I seem to remember someone telling me that case can never happen in practice so I have simply not worried about it. Perhaps I am misremembering this. Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate 2018-05-10 2:39 ` Eric W. Biederman @ 2018-05-10 2:39 ` Eric W. Biederman [not found] ` <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com> 1 sibling, 0 replies; 5+ messages in thread From: Eric W. Biederman @ 2018-05-10 2:39 UTC (permalink / raw) To: Matt Redfearn Cc: linux-arch, linux-kernel, Ralf Baechle, James Hogan, linux-mips Matt Redfearn <matt.redfearn@mips.com> writes: > Hi Eric, > > On 20/04/18 15:37, Eric W. Biederman wrote: >> Filling in struct siginfo before calling force_sig_info a tedious and >> error prone process, where once in a great while the wrong fields >> are filled out, and siginfo has been inconsistently cleared. >> >> Simplify this process by using the helper force_sig_fault. Which >> takes as a parameters all of the information it needs, ensures >> all of the fiddly bits of filling in struct siginfo are done properly >> and then calls force_sig_info. >> >> In short about a 5 line reduction in code for every time force_sig_info >> is called, which makes the calling function clearer. >> >> Cc: Ralf Baechle <ralf@linux-mips.org> >> Cc: James Hogan <jhogan@kernel.org> >> Cc: linux-mips@linux-mips.org >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >> --- >> arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- >> arch/mips/mm/fault.c | 19 ++++---------- >> 2 files changed, 23 insertions(+), 61 deletions(-) >> >> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c >> index 967e9e4e795e..66ec4b0b484d 100644 >> --- a/arch/mips/kernel/traps.c >> +++ b/arch/mips/kernel/traps.c >> @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) >> asmlinkage void do_ov(struct pt_regs *regs) >> { >> enum ctx_state prev_state; >> - siginfo_t info; >> - >> - clear_siginfo(&info); >> - info.si_signo = SIGFPE; >> - info.si_code = FPE_INTOVF; >> - info.si_addr = (void __user *)regs->cp0_epc; >> prev_state = exception_enter(); >> die_if_kernel("Integer overflow", regs); >> - force_sig_info(SIGFPE, &info, current); >> + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); >> exception_exit(prev_state); >> } >> @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) >> void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, >> struct task_struct *tsk) >> { >> - struct siginfo si; >> - >> - clear_siginfo(&si); >> - si.si_addr = fault_addr; >> - si.si_signo = SIGFPE; >> + int si_code; > > This is giving build errors in Linux next > (https://storage.kernelci.org/next/master/next-20180509/mips/defconfig+kselftest/build.log) > > si_code would have ended up as 0 before from the clear_siginfo(), but perhaps And si_code 0 is not a valid si_code to use with a floating point siginfo layout. > int si_code = FPE_FLTUNK; > > Would make a more sensible default? FPE_FLTUNK would make a more sensible default. I seem to remember someone telling me that case can never happen in practice so I have simply not worried about it. Perhaps I am misremembering this. Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com>]
* Re: [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate [not found] ` <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com> @ 2018-05-11 2:31 ` Eric W. Biederman 2018-05-11 2:31 ` Eric W. Biederman 0 siblings, 1 reply; 5+ messages in thread From: Eric W. Biederman @ 2018-05-11 2:31 UTC (permalink / raw) To: Matt Redfearn Cc: linux-arch, linux-kernel, Ralf Baechle, James Hogan, linux-mips Matt Redfearn <matt.redfearn@mips.com> writes: > Hi Eric, > > On 10/05/18 03:39, Eric W. Biederman wrote: >> Matt Redfearn <matt.redfearn@mips.com> writes: >> >>> Hi Eric, >>> >>> On 20/04/18 15:37, Eric W. Biederman wrote: >>>> Filling in struct siginfo before calling force_sig_info a tedious and >>>> error prone process, where once in a great while the wrong fields >>>> are filled out, and siginfo has been inconsistently cleared. >>>> >>>> Simplify this process by using the helper force_sig_fault. Which >>>> takes as a parameters all of the information it needs, ensures >>>> all of the fiddly bits of filling in struct siginfo are done properly >>>> and then calls force_sig_info. >>>> >>>> In short about a 5 line reduction in code for every time force_sig_info >>>> is called, which makes the calling function clearer. >>>> >>>> Cc: Ralf Baechle <ralf@linux-mips.org> >>>> Cc: James Hogan <jhogan@kernel.org> >>>> Cc: linux-mips@linux-mips.org >>>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >>>> --- >>>> arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- >>>> arch/mips/mm/fault.c | 19 ++++---------- >>>> 2 files changed, 23 insertions(+), 61 deletions(-) >>>> >>>> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c >>>> index 967e9e4e795e..66ec4b0b484d 100644 >>>> --- a/arch/mips/kernel/traps.c >>>> +++ b/arch/mips/kernel/traps.c >>>> @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) >>>> asmlinkage void do_ov(struct pt_regs *regs) >>>> { >>>> enum ctx_state prev_state; >>>> - siginfo_t info; >>>> - >>>> - clear_siginfo(&info); >>>> - info.si_signo = SIGFPE; >>>> - info.si_code = FPE_INTOVF; >>>> - info.si_addr = (void __user *)regs->cp0_epc; >>>> prev_state = exception_enter(); >>>> die_if_kernel("Integer overflow", regs); >>>> - force_sig_info(SIGFPE, &info, current); >>>> + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); >>>> exception_exit(prev_state); >>>> } >>>> @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) >>>> void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, >>>> struct task_struct *tsk) >>>> { >>>> - struct siginfo si; >>>> - >>>> - clear_siginfo(&si); >>>> - si.si_addr = fault_addr; >>>> - si.si_signo = SIGFPE; >>>> + int si_code; >>> >>> This is giving build errors in Linux next >>> (https://storage.kernelci.org/next/master/next-20180509/mips/defconfig+kselftest/build.log) >>> >>> si_code would have ended up as 0 before from the clear_siginfo(), but perhaps >> >> And si_code 0 is not a valid si_code to use with a floating point >> siginfo layout. >> >>> int si_code = FPE_FLTUNK; >>> >>> Would make a more sensible default? >> >> FPE_FLTUNK would make a more sensible default. >> >> I seem to remember someone telling me that case can never happen in >> practice so I have simply not worried about it. Perhaps I am >> misremembering this. > > It probably can't happen in practise - but the issue is that the > kernel doesn't even compile because -Werror=maybe-uninitialized > results in a build error since the compiler can't know that one of the > branches will definitely be taken to set si_code. My cross compile work. So I don't know where that -Werror=maybe-unitialized comes from. I agree it is an issue. I agree that FPE_FLTUNK is one of the good solutions. Another is to add a final else where you return without doing anything. Right now this looks like mips people issue that I have unearthed. I could appreciate some guidance on which way mips folks would like to handle this. If you can point me to where the fatal error is coming from I will definitely do something in my tree so that this is not a harmful issue. Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate 2018-05-11 2:31 ` Eric W. Biederman @ 2018-05-11 2:31 ` Eric W. Biederman 0 siblings, 0 replies; 5+ messages in thread From: Eric W. Biederman @ 2018-05-11 2:31 UTC (permalink / raw) To: Matt Redfearn Cc: linux-arch, linux-kernel, Ralf Baechle, James Hogan, linux-mips Matt Redfearn <matt.redfearn@mips.com> writes: > Hi Eric, > > On 10/05/18 03:39, Eric W. Biederman wrote: >> Matt Redfearn <matt.redfearn@mips.com> writes: >> >>> Hi Eric, >>> >>> On 20/04/18 15:37, Eric W. Biederman wrote: >>>> Filling in struct siginfo before calling force_sig_info a tedious and >>>> error prone process, where once in a great while the wrong fields >>>> are filled out, and siginfo has been inconsistently cleared. >>>> >>>> Simplify this process by using the helper force_sig_fault. Which >>>> takes as a parameters all of the information it needs, ensures >>>> all of the fiddly bits of filling in struct siginfo are done properly >>>> and then calls force_sig_info. >>>> >>>> In short about a 5 line reduction in code for every time force_sig_info >>>> is called, which makes the calling function clearer. >>>> >>>> Cc: Ralf Baechle <ralf@linux-mips.org> >>>> Cc: James Hogan <jhogan@kernel.org> >>>> Cc: linux-mips@linux-mips.org >>>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >>>> --- >>>> arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- >>>> arch/mips/mm/fault.c | 19 ++++---------- >>>> 2 files changed, 23 insertions(+), 61 deletions(-) >>>> >>>> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c >>>> index 967e9e4e795e..66ec4b0b484d 100644 >>>> --- a/arch/mips/kernel/traps.c >>>> +++ b/arch/mips/kernel/traps.c >>>> @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) >>>> asmlinkage void do_ov(struct pt_regs *regs) >>>> { >>>> enum ctx_state prev_state; >>>> - siginfo_t info; >>>> - >>>> - clear_siginfo(&info); >>>> - info.si_signo = SIGFPE; >>>> - info.si_code = FPE_INTOVF; >>>> - info.si_addr = (void __user *)regs->cp0_epc; >>>> prev_state = exception_enter(); >>>> die_if_kernel("Integer overflow", regs); >>>> - force_sig_info(SIGFPE, &info, current); >>>> + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); >>>> exception_exit(prev_state); >>>> } >>>> @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) >>>> void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, >>>> struct task_struct *tsk) >>>> { >>>> - struct siginfo si; >>>> - >>>> - clear_siginfo(&si); >>>> - si.si_addr = fault_addr; >>>> - si.si_signo = SIGFPE; >>>> + int si_code; >>> >>> This is giving build errors in Linux next >>> (https://storage.kernelci.org/next/master/next-20180509/mips/defconfig+kselftest/build.log) >>> >>> si_code would have ended up as 0 before from the clear_siginfo(), but perhaps >> >> And si_code 0 is not a valid si_code to use with a floating point >> siginfo layout. >> >>> int si_code = FPE_FLTUNK; >>> >>> Would make a more sensible default? >> >> FPE_FLTUNK would make a more sensible default. >> >> I seem to remember someone telling me that case can never happen in >> practice so I have simply not worried about it. Perhaps I am >> misremembering this. > > It probably can't happen in practise - but the issue is that the > kernel doesn't even compile because -Werror=maybe-uninitialized > results in a build error since the compiler can't know that one of the > branches will definitely be taken to set si_code. My cross compile work. So I don't know where that -Werror=maybe-unitialized comes from. I agree it is an issue. I agree that FPE_FLTUNK is one of the good solutions. Another is to add a final else where you return without doing anything. Right now this looks like mips people issue that I have unearthed. I could appreciate some guidance on which way mips folks would like to handle this. If you can point me to where the fatal error is coming from I will definitely do something in my tree so that this is not a harmful issue. Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-14 8:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <87604mhrnb.fsf@xmission.com>
2018-04-20 14:37 ` [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate Eric W. Biederman
[not found] ` <e0a5bde2-7817-3e05-1c8d-c8fa8f6aa5f2@mips.com>
2018-05-10 2:39 ` Eric W. Biederman
2018-05-10 2:39 ` Eric W. Biederman
[not found] ` <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com>
2018-05-11 2:31 ` Eric W. Biederman
2018-05-11 2:31 ` Eric W. Biederman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox