On Fri, Apr 20, 2012 at 07:07:48PM +0100, Al Viro wrote:
> On Fri, Apr 20, 2012 at 10:21:35AM -0700, Linus Torvalds wrote:
> > This is why I suggested you look at Oleg's patches. If we guarantee
> > that things won't be delayed past re-entering user mode, all those
> > issues go away.
>
> I've looked at them. One obvious problem is that it tracehook_notify_resume()
> is not universally called. AFAICS, hexagon, m68k, microblaze, score, um
> and xtensa never call tracehook_notify_resume(). Out of those, hexagon is
> manually checking TIF_NOTIFY_RESUME and does key_replace_session_keyring(),
> so the call could be easily added into the same place; the rest of those
> guys don't even look at TIF_NOTIFY_RESUME anywhere near their signal*.c
> and m68k/um/xtensa don't even have it defined, let alone handled. So this
> stuff depends on some amount of asm glue hacking on several architectures ;-/
BTW, I've looked into dealing with that; I think I have a tentative solution
for all these architectures.
* hexagon: just needs tracehook_notify_resume() added, everything
else is already in place
* score: TIF_NOTIFY_RESUME is defined *and* included into the
"we need to call do_notify_resume()" logics in assembler glue; just need
to add the usual boilerplate into said do_notify_resume()
* um: glue in question is in C; easily dealt with, I can do that
(and test the results) tonight
* m68k: that'll need some glue changes; AFAICS, the easiest solution
is to put TIF_NOTIFY_RESUME into bit 5 - then nommu glue needs no changes
at all, and entry_mm.S needs two "jmi do_signal_return" replaced with
"jne do_signal_return"; the code before those shifts bit 6 to MSBit and
currently bits 0--5 are unused. Replacing "most significant bit is set" with
"some bits are set" would do the right thing, AFAICT - make the sucker
go into do_signal() handling if either TIF_SIGPENDING (bit 6) or
TIF_NOTIFY_RESUME (bit 5) is set (at that point it has already checked that
TIF_NEED_RESCHEDULE is not set). On top of that it will need the obvious
changes in do_signal() itself - boilerplate added and current contents
made conditional on TIF_SIGPENDING being set. I can only test that
on aranym, though - all real m68k hardware I have is pining for fjords right
now.
* microblaze: TIF_NOTIFY_RESUME is defined, but not hooked anywhere.
Fortunately, the glue is easy enough there - all relevant spots have the
same form
lwi r11, r11, TI_FLAGS; /* get flags in thread info */
andi r11, r11, _TIF_SIGPENDING;
beqi r11, 1f; /* Signals to handle, handle them */
and replacing that _TIF_SIGPENDING with _TIF_SIGPENDING | _TIF_NOTIFY_RESUME
will do the right thing; of course, do_signal() itself will need to be
taught about TIF_NOTIFY_RESUME - same as in case of m68k. No hardware, no
emulators set up, but then it's less intrusive in the glue part than m68k
counterpart.
* xtensa: TIF_NOTIFY_RESUME needs to be defined (bit 7 would do,
AFAICS) and there the glue does need some change:
l32i a4, a2, TI_FLAGS
_bbsi.l a4, TIF_NEED_RESCHED, 3f
_bbci.l a4, TIF_SIGPENDING, 4f
should be replaced with (if I'm not misreading their ISA documentation)
l32i a4, a2, TI_FLAGS
_bbsi.l a4, TIF_NEED_RESCHED, 3f
_bbsi.l a4, TIF_SIGPENDING, 2f
_bbci.l a4, TIF_NOTIFY_RESUME, 4f
2:
(and do_signal() changes, of course). That's the most intrusive one and
again, I've neither hw nor emulators for that sucker.
I'll post the patches for all of those tonight; if everything ends up working,
at least we can get rid of the ifdefs on TIF_NOTIFY_RESUME.
Oleg, where does your tree live? I've walked through the signal handling
on all targets over this weekend (and it's still not complete - there are
fun bugs re multiple sigframes and restart handling on many of those) and
my current queue is at git.kernel.org/pub/scm/linux/kernel/git/viro/signal;
I don't promise that it even builds on everything, so it's *not* a pull
request. Besides, it's still growing and will be reordered... The issues
dealt with by now:
[done] don't open-code force_sigsegv()
[done] don't open-code block_sigmask()
[done] If we have failed to set sigframe up, we should send SIGSEGV with
force_sigsegv() (or force_sigsegv_info()) and leave the sigmask alone;
otherwise we need to call block_sigmask() and clear RESTORE_SIGMASK
[done] looking at SA_ONESHOT is pointless (it's a rudiment of very old things;
these days kernel/signal.c does it properly and arch/* code doesn't need to
and actually can't - it only gets a local copy filled, so clearing sa_handler
in it is bloody pointless)
[done] all sigsuspend variants should use sigsuspend() helper (i.e. be based
on use of ->saved_sigmask; see the first patch in queue introducing the helper
in question)
[done] restart_block.fn should be reset on sigreturn, not on signal delivery
[done] sigreturn variants should use set_current_blocked(); make sure to remove KILL/STOP from the set first
[mostly done; parisc and blackfin left] check __get_user()/__put_user() results
I really want to get arch/*/*/*signal* more or less in sync wrt fixes; having
TIF_NOTIFY_RESUME working fits nicely into that. I'd appreciate a look through
that stuff...
PS: I don't think I'll be posting any pull requests on that tree; it's just
a staging ground for future linux-arch patchbomb(s).
--
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
