linux-kernel - Re: [patch] nfs: fix locking in nfs/inode.c in nfs_free_open

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: <1185456203.6585.180.camel@localhost>
Date:	Thu, 26 Jul 2007 09:23:23 -0400
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Christian Krafft <krafft@...ibm.com>
Cc:	Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org
Subject: Re: [patch] nfs: fix locking in nfs/inode.c in
	nfs_free_open_context

On Thu, 2007-07-26 at 14:44 +0200, Christian Krafft wrote:
> On Thu, 26 Jul 2007 13:23:37 +0200
> Arnd Bergmann <arnd@...db.de> wrote:
> 
> > On Wednesday 25 July 2007, Trond Myklebust wrote:
> > > 
> > > On Wed, 2007-07-25 at 17:08 +0200, Christian Krafft wrote:
> > > 
> > > > Obviously the locking code in nfs_free_open_context is wrong.
> > > > Checking the list for entries and removing the entry should be an atomic operation.
> > > 
> > > Wrong. It is quite safe to test the structure member ctx->list for
> > > emptiness outside the spinlock because we have an explicit guarantee
> > > that nobody else has a reference to this structure, plus the
> > > atomic_dec_and_test() in kref_put() has acted as a memory barrier for
> > > us.
> > 
> > Well, the real question then is how the ctx can still be present in the
> > nfsi->open_files list. Since we are in nfs_free_open_context(), there
> > must not be any pointer to the ctx anywhere, but still we have this other
> > thread calling get_nfs_open_context() on it.
> > 
> > 	Arnd <><
> 
> Thanks for the pointer Arnd,
> 
> Trond, does the patch below look better to you ?

No. That is still incorrect. The list of open contexts is used for
things like NFSv4 state recovery (when we're doing background writes,
and the server happens to reboot). The lifetime of the open context may
exceed that of the struct file that created it.

Trond

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