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

[Oleg Nesterov]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/