linux-kernel - Re: workqueue question.

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: <20110630100031.GP3386@htj.dyndns.org>
Date:	Thu, 30 Jun 2011 12:00:31 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Ben Greear <greearb@...delatech.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: workqueue question.

Hello,

On Wed, Jun 29, 2011 at 09:02:29AM -0700, Ben Greear wrote:
> On 06/29/2011 01:43 AM, Tejun Heo wrote:
> It appears that the code just wants to (re)add itself to the
> work queue with a different callback method:
> 
> static void rpc_final_put_task(struct rpc_task *task,
> 		struct workqueue_struct *q)
> {
> 	if (q != NULL) {
> 		INIT_WORK(&task->u.tk_work, rpc_async_release);
> 		queue_work(q, &task->u.tk_work);
> 	} else
> 		rpc_free_task(task);
> }

Ummm... so, at the time of INIT_WORK(), the tk_work could be already
pending or running?

> My debugging leads me to believe that the rpc_async_release
> is (very rarely) called on a task object that has already been logically
> freed.

What do you mean "logically freed"?  Do you mean the rpc_task struct
is freed twice?

> Is there a better way to queue this up that might have less chance
> of some strange race?

Why not just use a separate work item?

> >>Also, is it valid to free the memory containing foo
> >>in a workqueue callback?
> >
> >Yeap.
> 
> Is there a method that can be called from a workqueue callback
> to verify that the item has not been re-added to the work-queue?

Can you be a bit more specific?  Are you saying that queue_work() and
INIT_WORK() may race?

> I tried doing a cancel, but that caused recursive locking issues.
> 
> I'd like to call this right before freeing the object and BUG_ON()
> if the object is actually still on on a work-queue.

That may be useful as a debugging feature but is inherently racy.
Nothing guarantees the work item won't be queued after BUG_ON() but
before actual freeing.  The guarantee that the work item is no longer
in use should come from the wq user.  There are good number of use
cases where work item frees itself or the containing data structure
and they all work fine.

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/