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.
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
