linux-kernel - Re: [PATCH v2] ipc/mqueue: remove STATE

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:	Tue, 28 Apr 2015 09:36:50 -0700
From:	Davidlohr Bueso <dave@...olabs.net>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Manfred Spraul <manfred@...orfullife.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Darren Hart <darren@...art.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	fredrik.markstrom@...driver.com,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH v2] ipc/mqueue: remove STATE_PENDING

On Tue, 2015-04-28 at 14:37 +0200, Peter Zijlstra wrote:
> On Mon, Apr 27, 2015 at 08:24:53PM -0700, Davidlohr Bueso wrote:
> > +static inline void pipelined_send(struct wake_q_head *wake_q,
> > +				  struct mqueue_inode_info *info,
> >  				  struct msg_msg *message,
> >  				  struct ext_wait_queue *receiver)
> >  {
> >  	receiver->msg = message;
> >  	list_del(&receiver->list);
> > +	wake_q_add(wake_q, receiver->task);
> > +	/*
> > +	 * Ensure that updating receiver->state is the last
> > +	 * write operation: As once set, the receiver can continue,
> > +	 * and if we don't have the reference count from the wake_q,
> > +	 * yet, at that point we can later have a use-after-free
> > +	 * condition and bogus wakeup.
> > +	 */
> > +	smp_wmb(); /* pairs with smp_rmb() in wq_sleep */
> 
> You have this barrier because we cannot rely on a failed cmpxchg()
> actually being a full barrier, right?

Failed cmpxchg() calls implies that the task is never added to the queue
(duplicate, which I cannot see occurring in this patch), so nothing
wrong with the bogus wakeups mentioned in the comment.

This barrier is not added by this patch though. Currently we have it
serializing with the wake_up_process() with STATE_READY, for similar
reasons. Because there is no task refcounting going on, the task can
easily disappear underneath us if the state is set before the wakeup. I
applied the same judgment here.

Thanks,
Davidlohr

> 
> >  	receiver->state = STATE_READY;
> >  }
> 
> 


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