CEPH-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Luis Henriques <lhenriques@suse.de>
Cc: ceph-devel@vger.kernel.org, linux-fscrypt@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH v5 19/19] ceph: add fscrypt ioctls
Date: Wed, 07 Apr 2021 08:47:58 -0400
Message-ID: <37ab61e4647c44a52947550db75191dc6cc94a30.camel@kernel.org> (raw)
In-Reply-To: <YGyiy1B+BaOQihrM@suse.de>

On Tue, 2021-04-06 at 19:04 +0100, Luis Henriques wrote:
> On Tue, Apr 06, 2021 at 01:27:21PM -0400, Jeff Layton wrote:
> <snip>
> > > > > I've spent a few hours already looking at the bug I reported before, and I
> > > > > can't really understand this code.  What does it mean to increment
> > > > > ->i_shared_gen at this point?
> > > > > 
> > > > > The reason I'm asking is because it looks like the problem I'm seeing goes
> > > > > away if I remove this code.  Here's what I'm doing/seeing:
> > > > > 
> > > > > # mount ...
> > > > > # fscrypt unlock d
> > > > > 
> > > > >   -> 'd' dentry is eventually pruned at this point *if* ->i_shared_gen was
> > > > >      incremented by the line above.
> > > > > 
> > > > > # cat d/f
> > > > > 
> > > > >   -> when ceph_fill_inode() is executed, 'd' isn't *not* set as encrypted
> > > > >      because both ci->i_xattrs.version and info->xattr_version are both
> > > > >      set to 0.
> > > > > 
> > > > 
> > > > Interesting. That sounds like it might be the bug right there. "d"
> > > > should clearly have a fscrypt context in its xattrs at that point. If
> > > > the MDS isn't passing that back, then that could be a problem.
> > > > 
> > > > I had a concern about that when I was developing this, and I *thought*
> > > > Zheng had assured us that the MDS will always pass along the xattr blob
> > > > in a trace. Maybe that's not correct?
> > > 
> > > Hmm, that's what I thought too.  I was hoping not having to go look at the
> > > MDS, but seems like I'll have to :-)
> > > 
> > 
> > That'd be good, if possible.
> > 
> > > > > cat: d/f: No such file or directory
> > > > > 
> > > > > I'm not sure anymore if the issue is on the client or on the MDS side.
> > > > > Before digging deeper, I wonder if this ring any bell. ;-)
> > > > > 
> > > > > 
> > > > 
> > > > No, this is not something I've seen before.
> > > > 
> > > > Dentries that live in a directory have a copy of the i_shared_gen of the
> > > > directory when they are instantiated. Bumping that value on a directory
> > > > should basically ensure that its child dentries end up invalidated,
> > > > which is what we want once we add the key to the directory. Once we add
> > > > a key, any old dentries in that directory are no longer valid.
> > > > 
> > > > That said, I could certainly have missed some subtlety here.
> > > 
> > > Great, thanks for clarifying.  This should help me investigate a little
> > > bit more.
> > > 
> > > [ And I'm also surprised you don't see this behaviour as it's very easy to
> > >   reproduce. ]
> > > 
> > > 
> > 
> > It is odd... fwiw, I ran this for 5 mins or so and never saw a problem:
> > 
> >     $ while [ $? -eq 0 ]; do sudo umount /mnt/crypt; sudo mount /mnt/crypt; fscrypt unlock --key=/home/jlayton/fscrypt-keyfile /mnt/crypt/d; cat /mnt/crypt/d/f; done
> > 
> 
> TBH I only do this operation once and it almost always fails.  The only
> difference I see is that I don't really use a keyfile, but a passphrase
> instead.  Not sure if it makes any difference.  Also, it may be worth
> adding a delay before the 'cat' to make sure the dentry is pruned.
> 

No joy. I tried different delays between 1-5s and it didn't change
anything.

> > ...do I need some other operations in between? Also, the cluster in this
> > case is Pacific. It's possible this is a result of changes since then if
> > you're on a vstart cluster or something.
> > 
> > $ sudo ./cephadm version
> > Using recent ceph image docker.io/ceph/ceph@sha256:9b04c0f15704c49591640a37c7adfd40ffad0a4b42fecb950c3407687cb4f29a
> > ceph version 16.2.0 (0c2054e95bcd9b30fdd908a79ac1d8bbc3394442) pacific (stable)
> 
> I've re-compiled the cluster after hard-resetting it to commit
> 6a19e303187c which you mentioned in a previous email in this thread.  But
> the result was the same.
> 
> Anyway, using a vstart cluster is also a huge difference I guess.  I'll
> keep debugging.  Thanks!
> 

I may try to set one up today to see if I can reproduce it. Thanks for
the testing help so far!

-- 
Jeff Layton <jlayton@kernel.org>


  reply index

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-26 17:32 [RFC PATCH v5 00/19] ceph+fscrypt: context, filename and symlink support Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 01/19] vfs: export new_inode_pseudo Jeff Layton
2021-04-08  1:08   ` Eric Biggers
2021-04-08 16:18     ` Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 02/19] fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode Jeff Layton
2021-04-08  1:06   ` Eric Biggers
2021-04-08 16:22     ` Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 03/19] fscrypt: export fscrypt_fname_encrypt and fscrypt_fname_encrypted_size Jeff Layton
2021-04-08  1:19   ` Eric Biggers
2021-03-26 17:32 ` [RFC PATCH v5 04/19] fscrypt: add fscrypt_context_for_new_inode Jeff Layton
2021-04-08  1:21   ` Eric Biggers
2021-04-08 16:27     ` Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 05/19] ceph: crypto context handling for ceph Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 06/19] ceph: implement -o test_dummy_encryption mount option Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 07/19] ceph: preallocate inode for ops that may create one Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 08/19] ceph: add routine to create fscrypt context prior to RPC Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 09/19] ceph: make ceph_msdc_build_path use ref-walk Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 10/19] ceph: add encrypted fname handling to ceph_mdsc_build_path Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 11/19] ceph: decode alternate_name in lease info Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 12/19] ceph: send altname in MClientRequest Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 13/19] ceph: properly set DCACHE_NOKEY_NAME flag in lookup Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 14/19] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 15/19] ceph: add helpers for converting names for userland presentation Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 16/19] ceph: add fscrypt support to ceph_fill_trace Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 17/19] ceph: add support to readdir for encrypted filenames Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 18/19] ceph: create symlinks with encrypted and base64-encoded targets Jeff Layton
2021-03-26 17:32 ` [RFC PATCH v5 19/19] ceph: add fscrypt ioctls Jeff Layton
2021-04-06 15:38   ` Luis Henriques
2021-04-06 16:03     ` Jeff Layton
2021-04-06 16:24       ` Luis Henriques
2021-04-06 17:27         ` Jeff Layton
2021-04-06 18:04           ` Luis Henriques
2021-04-07 12:47             ` Jeff Layton [this message]
2021-03-26 18:38 ` [RFC PATCH v5 00/19] ceph+fscrypt: context, filename and symlink support Jeff Layton
2021-03-31 20:35 ` [RFC PATCH v5 20/19] ceph: make ceph_get_name decrypt filenames Jeff Layton
2021-04-01 11:14   ` Luis Henriques
2021-04-01 12:15     ` Jeff Layton
2021-04-01 13:05       ` Luis Henriques
2021-04-01 13:12         ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37ab61e4647c44a52947550db75191dc6cc94a30.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=lhenriques@suse.de \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

CEPH-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/ceph-devel/0 ceph-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ceph-devel ceph-devel/ https://lore.kernel.org/ceph-devel \
		ceph-devel@vger.kernel.org
	public-inbox-index ceph-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.ceph-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git