From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:54098 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S933386AbcCKDS6 (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 10 Mar 2016 22:18:58 -0500
Date: Fri, 11 Mar 2016 03:18:51 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Theodore Ts'o <tytso@mit.edu>
Cc: "Drokin, Oleg" <oleg.drokin@intel.com>,
	"Dilger, Andreas" <andreas.dilger@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"<linux-fsdevel@vger.kernel.org>" <linux-fsdevel@vger.kernel.org>,
	Mark Fasheh <mfasheh@suse.com>
Subject: Re: races in ll_splice_alias() and elsewhere (ext4, ocfs2)
Message-ID: <20160311031851.GQ17997@ZenIV.linux.org.uk>
References: <498D5A19-9E55-48D7-B5CF-34CA5769FF7F@intel.com>
 <20160308211148.GX17997@ZenIV.linux.org.uk>
 <A5CA4682-77B1-4DF4-A784-7C7C2FAFC9AF@intel.com>
 <20160309003416.GY17997@ZenIV.linux.org.uk>
 <7C3EBB6F-54AC-4744-BEC1-33EA82216F85@intel.com>
 <20160309012658.GZ17997@ZenIV.linux.org.uk>
 <34C2B1C3-2B7F-490B-A03A-3BCDDFC8BE48@intel.com>
 <20160310022041.GF17997@ZenIV.linux.org.uk>
 <20160310025948.GG17997@ZenIV.linux.org.uk>
 <20160310235542.GC8890@thunk.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160310235542.GC8890@thunk.org>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Mar 10, 2016 at 06:55:43PM -0500, Theodore Ts'o wrote:

> The ext4_d_revalidate() function was an attempt to square the circle,
> but yes, I've been coming to the conclusion that it doesn't work all
> that well.  One thing I've been considering is to make access to the
> keys a global property.  So the first time a user with access to the
> key tries to access an encrypted file, we import the key into mounted
> file system's ext4_sb_info structure, and we bump a generation
> counter, and then ext4_d_revalidate() simply returns "invalid" for all
> denrty entries which (a) refer to an encrypted file or directory, and
> (b) whose generation number is less than the current generation
> number.

That sounds rather DoSable, if I'm not misparsing you...

> Similarly, if we invalidate a key, we remove the key from tthe keyring
> hanging off of the mounted file system's sb_info structure, and then
> bump the generation number, which will invalidate the dentries for all
> encrypted files/directories on that file system.
> 
> This is a bit non-optimal, but I don't see any other way of solving
> the problem.  Al, do you have any suggestions?

In any case, the current variant needs at least dget_parent()/dput() - blind
access of ->d_parent is simply broken.  As for using ->d_revalidate() for
that stuff...  I'm not sure.  How should those directories look like for
somebody who doesn't have a key?  Should e.g. getdents(2) results depend on
who's calling it, or who'd opened the directory, or...?