linux-kernel - Re: [PATCH 4/4] posix-timers: turn it_signal into it

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: <alpine.LFD.2.02.1109211835550.2723@ionos>
Date:	Wed, 21 Sep 2011 18:46:31 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Oleg Nesterov <oleg@...hat.com>
cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Andi Kleen <andi@...stfloor.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 4/4] posix-timers: turn it_signal into it_valid flag

On Wed, 7 Sep 2011, Oleg Nesterov wrote:
> On 09/06, Thomas Gleixner wrote:
> >
> > On Tue, 6 Sep 2011, Oleg Nesterov wrote:
> >
> > > But how this can help? Suppose that the task is preempted right
> > > after dequeue_signal() drops ->siglock. We need rcu_read_lock()
> > > before unlock then, no?
> >
> > Crap, you are right, but that's fortunately an easy to solve one :)
> 
> Yes, this is solvable. But I think we can do something better.
> 
> > > And. This breaks the accounting logic. I mean the patch from Andi
> > > which adds the limits.
> >
> > That's a different problem and really, it does not break it by any
> > means. When the timer is released, then the count is decreased and we
> > can safely assume that the memory is going to be freed in the next
> > grace period.
> 
> Yes, but this means we need the counter which we do not have.
> 
> I think we can avoid this problems. Although I am not sure, I am
> already sleeping.
> 
> 	- we add rcu_read_lock() into dequeueu_signal().
> 
> 	- we add the new "struct k_itimer *my_timer" member into
> 	 siginfo._timer. Like _sys_private it is not passed to
> 	 user, and perhaps we can kill _sys_private later.

sys_private is ugly as hell and we should avoid to add another field
to siginfo.

I think we can embed the timer siginfo into k_itimer instead and
provide a proper init function in signal.c. We always allocate a
siginfo for a timer so we are better of allocating that stuff at once.

That way we can use container_of() and check the timer fields
directly. That moves sys_private info k_itimer, gets rid of the idr
lookup and avoids an extra siginfo field.

Further this would allow us to distangle the posixtimer rlimit from
the sigpending rlimit, once we have that in place.

Thanks,

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