linux-kernel - Re: [patch] x86, ptrace: fix double-free on race

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:	Wed, 11 Feb 2009 23:08:21 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Markus Metzger <markus.t.metzger@...el.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, tglx@...utronix.de,
	hpa@...or.com, markus.t.metzger@...il.com, roland@...hat.com,
	eranian@...glemail.com, juan.villacis@...el.com
Subject: Re: [patch] x86, ptrace: fix double-free on race

On 02/11, Markus Metzger wrote:
>
> Ptrace_detach() races with __ptrace_unlink() if the traced task is
> reaped while detaching. This might cause a double-free of the BTS
> buffer.
>
> Change the ptrace_detach() path to only do the memory accounting in
> ptrace_bts_detach() and leave the buffer free to ptrace_bts_untrace()
> which will be called from __ptrace_unlink().

Thanks Markus, I think this should close the "really bad" problems
for 2.6.29.

Off-topic. This is subjective, so please feel to ignore, but personally
I dislike the usage of ptrace_fork() in copy_process(). And ptrace_fork()
itself.

To me, this has nothing to do with ptrace at all. copy_process() or
dup_task_struct() just must clear/tweak some fields after memcpy().

Perhaps it is better to kill all these ptrace_fork/arch_ptrace_fork/
x86_ptrace_fork/ptrace_bts_fork helpers, and make a single function

	static inline void arch_bts_init(struct task_struct *p)
	{
	#ifdef CONFIG_X86_PTRACE_BTS
		if (unlikely(p->bts)) {
			p->bts = NULL;
			p->bts_buffer = NULL;
			p->bts_size = 0;
			p->thread.bts_ovfl_signal = 0;
		}
	#endif /* CONFIG_X86_PTRACE_BTS */
	}

which is called by arch_dup_task_struct(). The "if (ptrace)" check
in copy_process() is just wrong imho, even if it is currently correct.
Let's suppose we add, say, bts_spinlock to the fields above. In that
case we must initialize it even if the forking task is not ptraced.

Of course, in any case this is not 2.6.29 material.

Btw, just curious, bts_ovfl_signal is not used, except the user can
set/get it via PTRACE_BTS_CONFIG/PTRACE_BTS_STATUS ?

Oleg.

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