Messages in this thread | | | Date | Tue, 29 May 2012 11:14:15 -0500 | From | Richard Kuo <> | Subject | Re: hexagon: signal bugs |
| |
On Sat, May 19, 2012 at 10:09:11PM +0100, Al Viro wrote: > 1) spot the braino: > void do_trap0(struct pt_regs *regs) > ... > unsigned long syscallret = 0; > ... > if ((unsigned long) regs->syscall_nr >= __NR_syscalls) { > regs->r00 = -1; > } else { > syscall = (syscall_fn) > (sys_call_table[regs->syscall_nr]); > syscallret = syscall(regs->r00, regs->r01, > regs->r02, regs->r03, > regs->r04, regs->r05); > } > > if (regs->syscall_nr != __NR_rt_sigreturn) > regs->r00 = syscallret; > IOW, the last if () should've been inside the "syscall_nr is not too high" > branch of the if () just above... > > 2) you do _not_ want syscall restarts to happen on sigreturn(). > Whatever adjustment needed to be done had already been done back before > we'd saved the registers into sigcontext. So doing > regs->syscall_nr = __NR_rt_sigreturn; > in sys_rt_sigreturn() is exactly the wrong thing - for syscall restart > purposes you want it to look like a non-syscall (i.e. it should be > regs->syscall_nr = -1 to make your check in handle_signal() work). > Incidentally, I would suggest just having that thing return regs->r00; > then you can avoid all special-casing for "do we want to put the > return value of sys_...() into regs->r00?" in do_trap0(). > > 3) if you have multiple pending signals, you need to handle all > of them before returning to userland. And you need to make sure that > syscall restart logics does not trigger on anything past the first time > around. > > 4) checking for !user_mode(regs) needs to be done in the loop (created > while fixing #3) in vm_entry.S where you are calling do_notify_resume(); > you need to leave the loop if it's true (if you can get there on return > to kernel in the first place).
Just wanted to say thanks so much for reviewing the code; I didn't get around to making the changes until late last week, but I've got the fixes in my tree and will submit a patch for review after I've given it some testing.
Thanks again, Richard Kuo
--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
| |