linux-kernel - Re: PATCH? tracehook_report

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: <20090601002226.480CEFC3C7@magilla.sf.frob.com>
Date:	Sun, 31 May 2009 17:22:26 -0700 (PDT)
From:	Roland McGrath <roland@...hat.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Christoph Hellwig <hch@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	linux-kernel@...r.kernel.org
Subject: Re: PATCH? tracehook_report_clone: fix false positives

> Firtsly, I don't understand CLONE_PTRACE check. Suppose that untraced
> task does clone(CLONE_PTRACE). In that case we create the untraced
> child (this is correct) but still we send SIGSTOP.
> 
> I do not really know if this bug or not, but this doesn't look right.
> At least this should be commented, imho. And, looking at 2.6.26, I think
> the behaviour was different before tracehooks.
> 
> So, I assume this is bug for now.

You're right.  CLONE_PTRACE when not traced will misbehave (not that anyone
ever uses it).  The old code just checked child->ptrace, and that is fine
to do again now.  I probably changed that thinking it had a race--which it
does--with asynchronous PTRACE_ATTACH after an untraced fork.  But that is
a harmless race as you explained.

ACK on the 2.6.30 patch attached.

> So, I am going to send the patch below. But this leads to another question:
> should not we move these sigaddset() + set_tsk_thread_flag() into
> ptrace_init_task() ?

It might make sense to consolidate them.  But note that ptrace_attach()
uses send_sig_info().  With SEND_SIG_FORCED, this does almost nothing more
than sigaddset() (i.e. no queue entry).  But it does do prepare_signal(),
which will clear any pending SIGCONTs.  It's possible that something in
userland manages to rely on that behavior for the asynchronous attach case
(unrelated to startup-time races).  It wouldn't hurt for the creation-time
case to use send_sig_info() too, though it would go through a bunch more
code to do nothing effectual but sigaddset() in the end.

Thanks,
Roland
--
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/