Re: [PATCH] ext4 crypto: migrate into vfs's crypto engine

[Jaegeuk Kim]

On Thu, May 05, 2016 at 08:44:12AM -0400, Theodore Ts'o wrote:
> On Wed, May 04, 2016 at 09:22:48PM -0700, Jaegeuk Kim wrote:
> > 
> > Got it. Let me add (*key_prefix(inode)) in fscrypt_operations so that filesystem
> > can give a specific prefix additionally.
> > Once fscrypto supports both of prefixes, does e4crypto have to set "fscrypto"?
> > The "ext4" should work all the time tho.
> 
> Well, the question is what userspace tool should be used for the
> non-Android case?
> 
> The other thing to consider is the cost of doing lookups in two
> keyrings all of the time.  I'm not sure how much overhead there is,
> but if it's significant, it might be time to hang a keyring off the
> superblock, and to start migrating to a model where keys are globally
> available to the file system once they get their (either implicitly
> because they are on a user's keyring, or explicitly via a new ioctl),
> and when they are removed (either implicitly because a key has been
> invalidated, or explicitly via a new ioctl), when a key gets added or
> removed from the file system global keyring, we bump a sequence
> counter so we can invalidate negative or positve dcache entries as
> appropriate.

The below patch implements allowing one more prefix given by fs.

Regarding to the overhead, I measured elapsed time of each function in
get_crypt_info() under x86 server.
And, I could see mostly negligible latencies including double check one.

Especially, the result shows just zero latencies except some cold misses
made by get_context(), derive_key_aes(), and crypto_alloc_skcipher(); those
are to read xattrs or load kernel crypto modules, I think.
After cold misses, I couldn't see any delay from get_crypt_info() actually.

Moreover, when considering prefix check is done by request_key() in early stage,
IMHO, calling it twice would not be a big deal.

Thought?

Thanks,