linux-kernel - Re: [patch 06/11] Use virtual debug registers in process/thread handling code

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: <20090310165828.GA22897@elte.hu>
Date:	Tue, 10 Mar 2009 17:58:28 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	prasad@...ux.vnet.ibm.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Roland McGrath <roland@...hat.com>
Subject: Re: [patch 06/11] Use virtual debug registers in process/thread
	handling code


* Alan Stern <stern@...land.harvard.edu> wrote:

> On Tue, 10 Mar 2009, Ingo Molnar wrote:
> 
> > > @@ -595,6 +596,12 @@ __switch_to(struct task_struct *prev_p, 
> > >  
> > >  	percpu_write(current_task, next_p);
> > >  
> > > +	/*
> > > +	 * Handle debug registers.  This must be done _after_ current
> > > +	 * is updated.
> > > +	 */
> > > +	if (unlikely(test_tsk_thread_flag(next_p, TIF_DEBUG)))
> > > +		switch_to_thread_hw_breakpoint(next_p);
> > 
> > why does this have to be called after 'current' has been 
> > updated? AFAICS switch_to_thread_hw_breakpoint() does not take a 
> > look at 'current'.
> 
> There was a discussion about this on LKML last October 17, and 
> you were in the CC list. [...]

I am on the Cc: list of thousands of messages per month. 
Consider it a very volatile form of storage.

Instead put these:

> There's a problem with moving the 
> switch_to_thread_hw_breakpoint() call before current is 
> updated.  Suppose a kernel breakpoint is triggered in between 
> the two.  The hw-breakpoint handler will see that current is 
> different from the task pointer stored in the chbi area, so it 
> will think the task pointer is leftover from an old task (lazy 
> switching) and will erase it.  Then until the next context 
> switch, no user-breakpoints will be installed.
> 
> The real problem is that it's impossible to update both 
> current and chbi->bp_task at the same instant, so there will 
> always be a window in which they disagree and a breakpoint 
> might get triggered.  Since we use lazy switching, we are 
> forced to assume that a disagreement means that current is 
> correct and chbi->bp_task is old.  But if you move the code 
> above then you'll create a window in which current is old and 
> chbi->bp_task is correct.

inside these:

  /*
   * ......
   */

Thanks,

        Ingo

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