linux-kernel - Re: [PATCH, for 2.6.29] ptrace: fix the usage of ptrace

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Feb 2009 22:48:04 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Markus Metzger <markus.t.metzger@...glemail.com>
Cc:	"Metzger, Markus T" <markus.t.metzger@...el.com>,
	Ingo Molnar <mingo@...e.hu>,
	Roland McGrath <roland@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH, for 2.6.29] ptrace: fix the usage of ptrace_fork()

On 02/10, Markus Metzger wrote:
>
> On Tue, 2009-02-10 at 21:21 +0100, Markus Metzger wrote:
> > On Tue, 2009-02-10 at 19:40 +0100, Oleg Nesterov wrote:
>
> > > Perhaps, for 2.6.29, we can do something like the "patch" below?
> > > 
> > > --- a/arch/x86/kernel/ptrace.c
> > > +++ b/arch/x86/kernel/ptrace.c
> > > @@ -810,11 +810,15 @@ static void ptrace_bts_untrace(struct ta
> > >  
> > >  static void ptrace_bts_detach(struct task_struct *child)
> > >  {
> > > +	// We can race with de_thread/do_wait which
> > > +	// can do ptrace_bts_untrace() before us
> > >  	if (unlikely(child->bts)) {
> > > -		ds_release_bts(child->bts);
> > > -		child->bts = NULL;
> > > -
> > > -		ptrace_bts_free_buffer(child);
> > > +		// This all will be freed by ptrace_bts_untrace()
> > > +		// later, but we should update ->mm
> > > +		down_write(->mmap_sem);
> > > +		mm->total_vm  -= bts_size;
> > > +		mm->locked_vm -= bts_size);
> > > +		up_write(->mmap_sem);
> > >  	}
> > >  }
> > >  #else
> > > 
> > 
>
> There's still a race.
> The kfree() is safe, now, but ptrace_bts_untrace() might have cleared
> child->bts_size before we can refund the memory.

Yes sure, please note the "We can race..." comment at the top
of ptrace_bts_detach().

The goal of this patch is to avoid the crash. The memory accounting
in ->mm is still not right. But at least, the tracer can not "steal"
the memory above the limits. And the "good" tracer should not exit
without detach, and it shouldn't release the tracee from sub-thread
if this can race with detach.

So, afaics, the worst thing which can happen is: the "bad" tracer
is punished by the "unfair" mm->xxx_vm numbers.

Except exec() can release the main thread whatever the tracer does...

> We need to make ptrace_bts_untrace() ignore child->bts_size and clear
> it in ptrace_bts_detach().

This is worse, now we can leak the memory if the tracer doesn't
do ptrace_detach().

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/