linux-kernel - Re: [PATCH 02/10] job control: introduce JOBCTL_TRAP

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]

Date:	Thu, 19 May 2011 12:19:30 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	jan.kratochvil@...hat.com, vda.linux@...glemail.com,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, indan@....nu, bdonlan@...il.com
Subject: Re: [PATCH 02/10] job control: introduce JOBCTL_TRAP_STOP and use
 it for group stop trap

Hello, Oleg.

On Wed, May 18, 2011 at 06:48:14PM +0200, Oleg Nesterov wrote:
> > Although this adds an unlock-relocking between checking of
> > JOBCTL_STOP_PENDING and actually trapping for STOP, this doesn't
> > affect correctness.
> 
> Well, I think it does affect. Although the problem is minor.
> 
> > ptrace_stop() already had conditional unlock-relocking
> 
> Yes, but ptrace_stop() can't send the CLD_STOPPED notfication before
> the tracee is ready for do_wait(WNOHANG). Contrary, get_signal_to_deliver()
> can if we race with SIGCONT.

That CLD_STOPPED notification is actually for continuation (although
it's indistinguishible from actual stopped notification) and as such
the ptracer has to query the tracee state after the notification
signal and can't expect it to be in TRACED.  Please consider the
following scenario.

1. thread t1 and t2.  t1 is ptraced.  Both running.

2. SIGSTOP delivered by t2.  Group stop starts.

3. SIGCONT is generated before t1 participates.

4. t1 will enter get_signal_to_deliver() and deliver CLD_STOPPED but
   won't trap.

So, I don't think this affects correctness.  The above can happen if
both t1 and t2 are attached by the same ptracer.  What changes is that
it now may happen with single thread too.  We can put STOP trap before
CONTINUED notification but I don't think that's necessary.

> > While at it, add proper function comment to do_signal_stop() and make
> > it return bool.
> > ...
> >
> > + * RETURNS:
> > + * %false if group stop is already cancelled and nothing happened.  %true
> > + * if participated in group stop.
> 
> Well, the traced task didn't participate yet... Nevermind.

Eh, should have read this earlier.  :-)

> So far I can't really understand why do we have both JOBCTL_TRAP_STOP and
> and JOBCTL_TRAPPING... Please ignore, I didn't read other patches yet.

JOBCTL_TRAP_STOP is the renamed JOBCTL_TRAP_INTERRUPT.  It's a trap
condition while TRAPPING is synchronization flag to protect -> TRACED
transitions.  JOBCTL_TRAP_STOP might not be the best name, but it
isn't INTERRUPT trap anymore JOBCTL_TRAP_GROUP_STOP or
JOBCTL_TRAP_GSTOP seemed a bit inconsistent with other flags.

> Hmm. And afaics there is a bug in do_signal_stop(), after
> "[PATCH 6/9] job control: make task_clear_jobctl_pending() clear TRAPPING automatically"
> ->jobctl &= ~JOBCTL_STOP_SIGMASK is no longer safe. We can clear _TRAPPING
> without wakeup.

Hmmm?  ->jobctl &= ~JOBCTL_STOP_SIGMASK clears lower sixteen bits.
All the flags including TRAPPING live in the upper sixteen bits.

Thank you.

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