linux-kernel - Re: 3.14-rc2 XFS backtrace because irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140213205146.GS18016@ZenIV.linux.org.uk>
Date:	Thu, 13 Feb 2014 20:51:46 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Dave Chinner <david@...morbit.com>,
	Dave Jones <davej@...hat.com>,
	Eric Sandeen <sandeen@...deen.net>,
	Linux Kernel <linux-kernel@...r.kernel.org>, xfs@....sgi.com
Subject: Re: 3.14-rc2 XFS backtrace because irqs_disabled.

On Wed, Feb 12, 2014 at 09:44:11PM +0000, Al Viro wrote:

> I'll try to put something along those lines together, if you or Oleg don't
> do it first.

OK, having looked at that stuff...

1) things become much more compact if we finish conversion to get_signal()
first.  Callers of get_signal_to_deliver() have k_sigaction and siginfo in
pair of local variables; switching to ksignal will be neutral wrt stack
footprint (it just gathers those two in one struct) *and* we are getting
rid of passing struct siginfo * around.  With that done, we can change
struct ksignal ->info with zero impact on the code in arch/*, and conversion
makes sense on its own.  In the mainline we have it done for alpha, arm,
openrisc, sparc and x86.  I've just put together preliminary (and completely
untested) patches for arm64, m68k and um; doing the rest won't take long, but
they'll obviously need to be tested.  It's a fairly safe conversion;
I'd expect the worst bugs to be typos.

2) struct small_siginfo ought to go into linux/signal.h.  Contains signal
number, si_code (usually 0), uid, pid and (often NULL) pointer to
struct sigqueue.  Overall: 20 or 24 bytes.  In struct coredump_params we
should replace ->siginfo with pointer to struct small_siginfo.  Ditto for
task_struct ->last_siginfo.  In struct ksignal ->info becomes
struct small_siginfo.

copy_siginfo_to_user{,32}() gets switched to struct small_siginfo *, so does
do_coredump(), ptrace_signal(), ptrace_stop(), ptrace_getsiginfo(),
ptrace_setsiginfo(), collect_signal(), __dequeue_signal(), dequeue_signal(),
signalfd_dequeue() and do_sigtimedwait().

New primitive: assign_sigqueue(small_info, sigqueue).  Frees
small_info->q and assigns a new value to it.

When ptrace_setsiginfo() is given a non-plain siginfo (si_code != SI_USER,
that is), it should a allocate struct sigqueue, copy the sucker there and
give it to assign_sigqueue().  Plain ones just get uid/pid/signo copied
and NULL passed to assign_sigqueue().

code in ptrace_signal() under if (signr != info->si_signo) should
start with assign_sigqueue(info, NULL).

specific_send_sig_info() in ptrace_signal() is rather clumsy to handle; not
sure what's the best way to deal with that.  In any case, ptrace_signal()
deciding to return 0 should take care of info->q - either copy and free the
original, or actually try to reuse the sucker.  Either way, info->q should
become NULL.

get_signal_to_deliver() should do assign_sigqueue(info, NULL) before
the call of do_group_exit().

arch_ptrace_stop_needed() can be left as-is; it's a macro and none of the
instances look at the info argument at all.  The same goes for
arch_ptrace_stop().

signalfd_read() should do assign_sigqueue(&info, NULL) right after
signalfd_copyinfo().

rt_sigtimedwait(2) and its compat variant should do the same right after
copy_siginfo_to_user{,32}().

TP'ed stuff is a mess, as usual.  AFAICS, TP_STORE_SIGINFO() needs to
be split into a variant taking siginfo (on trace_signal_generate side)
and one taking small_siginfo (for trace_signal_deliver).

get_signal() should do assign_sigqueue(&ksig->info, NULL) if it returns
false and use task_work_add() to schedule info->q for freeing otherwise.

Overall, aside of the need to complete arch/* conversion, the only unfinished
part is this:
        /* If the (new) signal is now blocked, requeue it.  */
        if (sigismember(&current->blocked, signr)) {
                specific_send_sig_info(signr, info, current);
                signr = 0;
        }
in the end of ptrace_signal().  Sure, we can add a local struct siginfo
in that if, fill it if needed and pass its address or &info->q->info
to specific_send_sig_info(), but that feels kludgy...  Hell knows, I'll
look a bit more into that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/