linux-kernel - Re: workqueue code needing preemption disabled

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: <1363633050.25967.210.camel@gandalf.local.home>
Date:	Mon, 18 Mar 2013 14:57:30 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Tejun Heo <tj@...nel.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	RT <linux-rt-users@...r.kernel.org>,
	Clark Williams <clark@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: workqueue code needing preemption disabled

On Mon, 2013-03-18 at 11:21 -0700, Tejun Heo wrote:

> I've been thinking about it and AFAICS the only way that BUG_ON()
> could trigger from preemption is if preemption happens while the
> idle_list head is becoming or stopping being empty.
> ie. pool->worklist is half updated so list_empty() isn't true but the
> first next entry is already pointing back to itself.  If there's a
> crashdump, it shouldn't be too difficult to verify and wrapping the
> above two functions should resolve it.

I like the theory, but it has one flaw. I agree that the update should
be wrapped in preempt_disable() but since this bug happens on the same
CPU, the state of the list will be the same when it was preempted to
when it bugged. That said:

static inline int list_empty(const struct list_head *head)
{
	return head->next == head;
}

That means when the task was preempted, head->next will either be
pointing to the next element or back to the list head. Which means if we
get preempted while updating the list, it will either see the head->next
== head or head->next == the next element.

first_worker() returns list_first_entry() which returns head->next. I
can't see how it would see the list_head and have list_empty() return
false.

-- Steve

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