linux-kernel - Re: 2.6.24-rc2-mm1: kcryptd vs lockdep

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: <1195661173.12000.46.camel@johannes.berg>
Date:	Wed, 21 Nov 2007 17:06:13 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	Alasdair G Kergon <agk@...hat.com>, Milan Broz <mbroz@...hat.com>,
	Ingo Molnar <mingo@...e.hu>,
	Torsten Kaiser <just.for.lkml@...glemail.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: 2.6.24-rc2-mm1: kcryptd vs lockdep

Hi,

> > Ok, then I have question: Is the following pseudocode correct
> > (and problem is in lock validation which checks something
> > already initialized for another queue) or reusing work_struct
> > is not permitted from inside called work function ?
> >
> > (Note comment in code "It is permissible to free the struct
> > work_struct from inside the function that is called from it".)
> >
> > struct work_struct work;
> > struct workqueue_struct *a, *b;
> >
> > do_b(*work)
> > {
> >         /* do something else */
> > }
> >
> > do_a(*work)
> > {
> >         /* do something */
> >         INIT_WORK(&work, do_b);
> >         queue_work(b, &work);
> > }
> >
> >
> > INIT_WORK(&work, do_a);
> > queue_work(a, &work);
> 
> (just in case, in that particular case PREPARE_WORK() should be used)
> 
> INIT_WORK(w) can be used if we know that "w" is not pending, and nobody
> else can write to this work (say, queue_work(w) or cancel_work_sync(w)).
> So currently the code above should work correctly.
> 
> However, I'd say it is not correct, INIT_WORK() can throw out some debug
> info for example, or the implementation could be changed.
> 
> I'm not sure about CONFIG_LOCKDEP (Johannes cc'ed). INIT_WORK() does
> lockdep_init_map(->lockdep_map) but run_workqueue() has a local copy,
> looks ok.

We explicitly need to use a copy of the lockdep_map for "locking" the
work struct as per the quoted comment. So as far as I can tell, what
INIT_WORK() is doing here is changing an at that point unused copy of
the lockdep map so I think it should be fine. Not sure about the other
fine points nor why you'd want this though :)

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)