linux-kernel - Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs

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]

Date:	Wed, 1 Dec 2010 11:52:17 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Hugh Dickins <hughd@...gle.com>
Cc:	Trond Myklebust <Trond.Myklebust@...app.com>,
	Nick Bowler <nbowler@...iptictech.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-nfs@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Christoph Hellwig <hch@....de>,
	Al Viro <viro@...iv.linux.org.uk>,
	Nick Piggin <npiggin@...nel.dk>
Subject: Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir

On Wed, Dec 1, 2010 at 11:23 AM, Hugh Dickins <hughd@...gle.com> wrote:
>
> But I'd prefer us not to throw in another callback if it's well
> workable with the ->releasepage we already have.

The thing is, NFS already really uses releasepage for its "real"
designed usage, in the form of the fscache case (which the directory
inodes don't use).

I really hate mixing things up. The NFS directory case really hasn't
got anything to do with releasepage, and taking the page lock on the
read side is just really sad. As is marking the page not up-to-date,
just so that it will get filled again.

In fact, I don't even know if it's kosher by VFS standards to clear
the up-to-date bit in the first place after it has already gotten set.
It absolutely isn't for anything that can be mmap'ed. Obviously, mmap
doesn't work on the directory cache anyway, but the point is that the
work-arounds for the semantic gap are really quite ugly.

Certainly much uglier than just adding some much simpler semantics to
the "now I'm releasing the page" case in the VM.

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