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

[Metzger, Markus T]

>-----Original Message-----
>From: Oleg Nesterov [mailto:oleg@xxxxxxxxxx]
>Sent: Wednesday, February 11, 2009 11:08 PM
>To: Metzger, Markus T


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

It looked cleaner and more consistent to me, that way.

Ptrace passes things down to arch in other cases already,
e.g. ptrace_detach()->ptrace_disable(), ptrace()->arch_ptrace(),
ptrace_attach()->arch_ptrace_attach().

I think the function should be in arch/x86/ptrace.c - if we call it from
ptrace or from arch_dup_task_struct() doesn't really matter.

Do you want me to move the call to arch_dup_task_struct()?


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

You're right, it is not strictly necessary and would actually be in the
way if we added more fields. I'm OK to remove it.


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

It's not really necessary for debugging, but we wanted to get the interface
general enough to support other use cases.

There's already a PMI interrupt handler in perfmon which handles the PEBS
buffer overflow. Stephane was planning to extract it so it can be used by
ds.c. Once that is done, we were planning to move the buffer overflow
handling for BTS and PEBS into ds.c and enable the feature in ptrace.


regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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