From: Al Viro <viro@ZenIV.linux.org.uk>
To: Richard Kuo <rkuo@codeaurora.org>
Cc: linux-kernel@vger.kernel.org, linux-hexagon@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: hexagon: signal handling bugs
Date: Sat, 11 Feb 2012 23:27:40 +0000 [thread overview]
Message-ID: <20120211232740.GM23916@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20110817163521.165013232@codeaurora.org>
1) unless I'm seriously misreading hexagon vm_entry.S, once we'd
called do_notify_resume, we do *not* recheck if there's more work to
be done. In particular, if we'd got more signals pending - e.g. from
SIGSEGV we'd got while trying to set a sigframe up. That's not a good
thing for a lot of reasons. Note that once that is fixed, you'll need
to make sure that restart logics is applied only *once* - not for subsequent
sigframes.
2) sigreturn should *NOT* be restartable at all, or you'll get
a lot of hard-to-reproduce fun. AFAICS, the right thing to do would
be to set ->syscall_nr to something negative, not __NR_rt_sigreturn in there
(and return regs->r00 instead of 0, to avoid the check in do_trap0).
Reason: suppose you have e.g. a loop that runs through different values in
r00. No syscalls in it. A signal comes and gets treated on the way
out of kernel after the next e.g. timer interrupt. We save the values
of registers into a sigframe and go into the handler. Another signal
comes; the next time we reach the kernel is when we get to rt_sigreturn().
There we
* have registers restored from on-stack sigframe
* have ->syscall_nr set to __NR_rt_sigreturn
* hit handle_signal() on the way out
* check ->r00; note that this is the value we have just restored
from sigframe. If it happens to be -ERESTARTNOHAND, silently replace it
with -EINTR. And eventually return to clueless userland.
Result: if timer interrupt happens when the userland loop goes through
-ERESTARTNOHAND, the value of r00 is clobbered upon return from signal handler.
Fun debugging that kind of bugs: priceless...
3) on sigreturn we need to set current->restart_block.fn; hexagon
doesn't do that.
4) altstack can overflow; you don't check that. See
commit 83bd01024b1fdfc41d9b758e5669e80fca72df66
Author: Roland McGrath <roland@redhat.com>
Date: Wed Jan 30 13:30:06 2008 +0100
for an analog.
5) try_to_freeze() has no business being called in do_signal();
let get_signal_to_deliver() handle that.
I don't have any access to hardware *or* cross-toolchain, so all I can do
here is RTFS and review. For #2..#5 I would not be afraid to post untested
patches (they are essentially trivial), but #1 is well beyond my arrogance
threshold - messing in assembler glue on an architecture I don't know *and*
can't test... Sorry.
next prev parent reply other threads:[~2012-02-11 23:27 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-17 16:34 [patch 00/36] Hexagon: Add support for Qualcomm Hexagon architecture Richard Kuo
2011-08-17 16:34 ` [patch 01/36] Hexagon: Add generic headers Richard Kuo
2011-08-17 19:19 ` Arnd Bergmann
2011-08-17 16:34 ` [patch 02/36] Hexagon: Core arch-specific header files Richard Kuo
2011-08-17 19:19 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 03/36] Hexagon: Add bitops support Richard Kuo
2011-08-17 19:19 ` Arnd Bergmann
2011-08-26 20:34 ` Pavel Machek
2011-08-30 21:14 ` ARM assembly syntax (was Re: [patch 03/36] Hexagon: Add bitops support) Linas Vepstas (Code Aurora)
2011-09-02 16:53 ` Pavel Machek
2011-08-30 21:59 ` [patch 03/36] Hexagon: Add bitops support Måns Rullgård
2011-08-17 16:35 ` [patch 04/36] Hexagon: Add atomic ops support Richard Kuo
2011-08-17 19:20 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 05/36] Hexagon: Add syscalls Richard Kuo
2011-08-17 19:29 ` Arnd Bergmann
2011-08-17 20:12 ` Jonas Bonn
2011-08-17 16:35 ` [patch 06/36] Hexagon: Add processor and system headers Richard Kuo
2011-08-17 19:31 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 07/36] Hexagon: Add threadinfo Richard Kuo
2011-08-17 19:37 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 08/36] Hexagon: Add delay functions Richard Kuo
2011-08-17 19:38 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 09/36] Hexagon: Add checksum functions Richard Kuo
2011-08-17 19:40 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 10/36] Hexagon: Add memcpy and memset accelerated functions Richard Kuo
2011-08-17 16:35 ` [patch 11/36] Hexagon: Add hypervisor interface Richard Kuo
2011-08-17 16:35 ` [patch 12/36] Hexagon: Export ksyms defined in assembly files Richard Kuo
2011-08-17 16:35 ` [patch 13/36] Hexagon: Support dynamic module loading Richard Kuo
2011-08-17 19:41 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 14/36] Hexagon: Add signal functions Richard Kuo
2012-02-11 23:27 ` Al Viro [this message]
2012-02-15 17:45 ` hexagon: signal handling bugs Richard Kuo
2012-02-15 18:18 ` Linas Vepstas
2011-08-17 16:35 ` [patch 15/36] Hexagon: Add init_task and process functions Richard Kuo
2011-08-17 19:45 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 16/36] Hexagon: Add startup code Richard Kuo
2011-08-17 16:35 ` [patch 17/36] Hexagon: Add interrupts Richard Kuo
2011-08-17 16:35 ` [patch 18/36] Hexagon: Add time and timer functions Richard Kuo
2011-08-17 16:35 ` [patch 19/36] Hexagon: Add ptrace support Richard Kuo
2011-08-17 19:47 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 20/36] Hexagon: Provide basic debugging and system trap support Richard Kuo
2011-08-17 16:35 ` [patch 21/36] Hexagon: Add SMP support Richard Kuo
2011-08-17 16:35 ` [patch 22/36] Hexagon: Add locking types and functions Richard Kuo
2011-08-17 16:35 ` [patch 23/36] Hexagon: Add user access functions Richard Kuo
2011-08-17 16:35 ` [patch 24/36] Hexagon: Provide basic implementation and/or stubs for I/O routines Richard Kuo
2011-08-17 19:55 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 25/36] Hexagon: Implement basic cache-flush support Richard Kuo
2011-08-17 16:35 ` [patch 26/36] Hexagon: Implement basic TLB management routines for Hexagon Richard Kuo
2011-08-17 16:35 ` [patch 27/36] Hexagon: Provide DMA implementation Richard Kuo
2011-08-17 20:01 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 28/36] Hexagon: Add ioremap support Richard Kuo
2011-08-17 16:35 ` [patch 29/36] Hexagon: Add page table header files & etc Richard Kuo
2011-08-17 16:35 ` [patch 30/36] Hexagon: Add page-fault support Richard Kuo
2011-08-17 16:35 ` [patch 31/36] Hexagon: kgdb support files Richard Kuo
2011-08-17 16:35 ` [patch 32/36] Hexagon: Comet platform support Richard Kuo
2011-08-17 20:07 ` Arnd Bergmann
2011-08-17 23:45 ` Linas Vepstas
2011-08-17 20:08 ` David Brown
2011-08-17 16:35 ` [patch 33/36] Hexagon: Platform-generic support Richard Kuo
2011-08-17 20:20 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 34/36] Hexagon: Add configuration and makefiles for the Hexagon architecture Richard Kuo
2011-08-17 20:27 ` Arnd Bergmann
2011-08-17 16:35 ` [patch 35/36] Hexagon: Add basic stacktrace functionality for " Richard Kuo
2011-08-17 16:35 ` [patch 36/36] Hexagon: Add self to MAINTAINERS Richard Kuo
2011-08-17 19:00 ` [patch 00/36] Hexagon: Add support for Qualcomm Hexagon architecture Zan Lynx
2011-08-17 19:42 ` Richard Kuo
2011-08-17 20:34 ` Arnd Bergmann
2011-08-18 0:31 ` Richard Kuo
2011-08-31 5:48 ` Avi Kivity
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120211232740.GM23916@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-hexagon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rkuo@codeaurora.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).