linux-kernel - Re: [PATCH 08/15] knfsd: spawn kernel thread to probe callback channel

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] [day] [month] [year] [list]

Date:	Mon, 27 Aug 2007 19:36:15 -0400
From:	"J. Bruce Fields" <bfields@...ldses.org>
To:	Neil Brown <neilb@...e.de>
Cc:	linux-kernel@...r.kernel.org, nfs@...ts.sourceforge.net,
	Adrian Bunk <bunk@...nel.org>
Subject: Re: [PATCH 08/15] knfsd: spawn kernel thread to probe callback
	channel

On Tue, Aug 28, 2007 at 09:26:36AM +1000, Neil Brown wrote:
> On Monday August 27, bfields@...ldses.org wrote:
> >  
> > +/* Reference counting, callback cleanup, etc., all look racy as heck.
> > + * And why is cb_set an atomic? */
> 
> Agreed.... so do we really want this code in mainline?  is the old
> code so bad that this is better?

The code in question is just moved, not rewritten, so those are
preexisting problems that I was just adding a whiny comment about in
hopes it would remind me some more patches were probably needed.
(Thanks for the reminder....)

>  - cb_set should not be atomic.
>  - This looks like a job for async-rpc rather than a kernel thread

The problem isn't the wait for the rpc itself, it's the creation of the
new cred that's needed in the gss case.  That could also be made
asynchronous, but as there's several other delicate changes in the
pipeline (keyring stuff, etc.), I'd rather not touch it for now.

>  - If you do use a thread, you at least want __module_get before
>    starting the thread, and module_put_and_exit to terminate the
>    thread.
>  - Can you just use 'cb_client' rather than cb_set?  If you move
>    rpc_create into the thread, you don't need to set cb_client until
>    the callback is successful.  Then add a 'cb_active' flag bit
>    so that you don't have two callbacks at the same time, and it
>    should be less racy..

Yep, probably so.

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