linux-kernel - Re: [PATCH 4/6] signal: sigprocmask() should do retarget_shared

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: <20110412183314.GA16342@mtj.dyndns.org>
Date:	Wed, 13 Apr 2011 03:33:14 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Nikita V. Youshchenko" <nyoushchenko@...sta.com>,
	Matt Fleming <matt@...sole-pimps.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] signal: sigprocmask() should do
 retarget_shared_pending()

Hello, Oleg.

On Mon, Apr 11, 2011 at 07:21:37PM +0200, Oleg Nesterov wrote:
> --- sigprocmask/include/linux/signal.h~4_sigprocmask_retarget	2011-04-06 21:33:50.000000000 +0200
> +++ sigprocmask/include/linux/signal.h	2011-04-11 18:16:51.000000000 +0200
> @@ -126,10 +126,14 @@ _SIG_SET_BINOP(sigandsets, _sig_and)
>  #define _sig_nand(x,y)	((x) & ~(y))
>  _SIG_SET_BINOP(signandsets, _sig_nand)
>  
> +#define _sig_nor(x,y)	((x) | ~(y))
> +_SIG_SET_BINOP(signorsets, _sig_nor)
> +
>  #undef _SIG_SET_BINOP
>  #undef _sig_or
>  #undef _sig_and
>  #undef _sig_nand
> +#undef _sig_nor

I'm confused.  Isn't nand ^(A&B) and nor ^(A|B)?

>  #define _SIG_SET_OP(name, op)						\
>  static inline void name(sigset_t *set)					\
> --- sigprocmask/kernel/signal.c~4_sigprocmask_retarget	2011-04-10 21:57:42.000000000 +0200
> +++ sigprocmask/kernel/signal.c	2011-04-11 18:02:22.000000000 +0200
> @@ -2131,6 +2131,11 @@ int sigprocmask(int how, sigset_t *set, 
>  	}
>  
>  	spin_lock_irq(&tsk->sighand->siglock);
> +	if (signal_pending(tsk) && !thread_group_empty(tsk)) {
> +		sigset_t not_newblocked;
> +		signorsets(&not_newblocked, &current->blocked, &newset);
> +		retarget_shared_pending(tsk, &not_newblocked);

I think it would be much easier to follow the logic if
retarget_shared_pending() took target mask instead of blocked
(ie. negation of blocked) and there were more comments.  Combined with
the confusing definition of nor, I had to spend quite some time trying
to wrap my head around it but the logic here isn't all that complex
and it should have been easier.

Other than that, I agree with the proposed changes, but I think we
really need to do retargeting (and the initial targeting too) more
efficiently as you noted in the earlier commit message.  Altering
signal mask is far hotter path than actual signal delivery.  Although
the thread walking code wouldn't get activated unless signal is
already pending, I still think we better avoid looping threads
unnecessarily.

Thanks.

-- 
tejun
--
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/