linux-kernel - Re: [PATCH 25/25] ipc: don't use [delayed_]work

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: <20121225032914.GE10220@mtj.dyndns.org>
Date:	Mon, 24 Dec 2012 19:29:14 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Borislav Petkov <bp@...en8.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 25/25] ipc: don't use [delayed_]work_pending()

Hello,

On Mon, Dec 24, 2012 at 08:41:01PM +0100, Borislav Petkov wrote:
> On Mon, Dec 24, 2012 at 10:45:20AM -0800, Tejun Heo wrote:
> > I was confused a bit there. We can't. Nothing guarantees that the
> > queuer sees the cleared PENDING before the work item starts execution,
> > and I think ipc memory hotplug could also be broken from that.
> 
> Stupid question: why not clear PENDING after execution is done? I'm
> looking at process_one_work() here.

The behavior is primarily because we want to enable workqueue users to
guarantee that a full work item execution happens at least once after
certain event happens.  ie. Let's say there's a work item which
processes data generated by a device.  If it's IRQ handler calls
queue_work() after getting notified of new data segment by the device,
it would want to guarantee that whole work item execution would happen
afterwards.  If you clear PENDING after execution, the event may
overlap with the end of the previous execution and the new data won't
be processed.

> > It's highly unlikely to actually happen and there may be external
> > locking which prevents the race from actually happening, but there's
> > nothing synchronizing queueing and the execution of the work item.
> > Looking at that part of code only, it's possible that it fails to
> > queue the work item after a memory hotplug event even though the
> > previous queueing already started execution and processed a couple
> > notifiers.
> 
> Maybe failure to queue could be signalled with a proper return value
> from __queue_work()?

It already does that but it's not about the return value.  You simply
cannot know whether to proceed with queueing or not with test_bit()
alone.

> Btw, I'm afraid I don't understand the "memory hotplug event" aspect and
> how that can influence the queueing - all it does it is list_add_tail,
> basically.

It doesn't have anything to do with memory hotplug event itself.  It's
a generic memory access ordering / synchronization issue.  There just
isn't anything preventing test_bit() from seeing PENDING bit from
before clearing.

Thanks.

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