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 20/19] ceph: make ceph_get_name decrypt filenames
Date: Thu, 01 Apr 2021 08:15:51 -0400
Message-ID: <8df5d18a65be8385f915dd7f3655db90d905b7c7.camel@kernel.org> (raw)
In-Reply-To: <YGWrKxYOdWgrhOPp@suse.de>

On Thu, 2021-04-01 at 12:14 +0100, Luis Henriques wrote:
> On Wed, Mar 31, 2021 at 04:35:20PM -0400, Jeff Layton wrote:
> > When we do a lookupino to the MDS, we get a filename in the trace.
> > ceph_get_name uses that name directly, so we must properly decrypt
> > it before copying it to the name buffer.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/export.c | 42 +++++++++++++++++++++++++++++++-----------
> >  1 file changed, 31 insertions(+), 11 deletions(-)
> > 
> > This patch is what's needed to fix the "busy inodes after umount"
> > issue I was seeing with xfstest generic/477, and also makes that
> > test pass reliably with mounts using -o test_dummy_encryption.
> 
> You mentioned this issue the other day on IRC but I couldn't reproduce.
> 
> On the other hand, I'm seeing another issue.  Here's a way to reproduce:
> 
> - create an encrypted dir 'd' and create a file 'f'
> - umount and mount the filesystem
> - unlock dir 'd'
> - cat d/f
>   cat: d/2: No such file or directory

I assume the message really says "cat: d/f: No such file or directory"

> 
> It happens _almost_ every time I do the umount+mount+unlock+cat.  Looks
> like ceph_atomic_open() fails to see that directory as encrypted.  I don't
> think the problem is on this open itself, but in the unlock because a
> simple 'ls' also fails to show the decrypted names.  (On the other end, if
> you do an 'ls' _before_ the unlock, everything seems to work fine.)
> 
> I didn't had time to dig deeper into this yet, but I don't remember seeing
> this behaviour in previous versions of the patchset.
> 
> Cheers,
> --
> Luís
> 

I've tried several times to reproduce this, but I haven't seen it happen
at all. It may be dependent on something in your environment (MDS
version, perhaps?). I'll try some more, but let me know if you track
down the cause.

Thanks,
Jeff

> > 
> > diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> > index 17d8c8f4ec89..f4e3a17ffc01 100644
> > --- a/fs/ceph/export.c
> > +++ b/fs/ceph/export.c
> > @@ -7,6 +7,7 @@
> >  
> >  #include "super.h"
> >  #include "mds_client.h"
> > +#include "crypto.h"
> >  
> >  /*
> >   * Basic fh
> > @@ -516,7 +517,9 @@ static int ceph_get_name(struct dentry *parent, char *name,
> >  {
> >  	struct ceph_mds_client *mdsc;
> >  	struct ceph_mds_request *req;
> > +	struct inode *dir = d_inode(parent);
> >  	struct inode *inode = d_inode(child);
> > +	struct ceph_mds_reply_info_parsed *rinfo;
> >  	int err;
> >  
> >  	if (ceph_snap(inode) != CEPH_NOSNAP)
> > @@ -528,29 +531,46 @@ static int ceph_get_name(struct dentry *parent, char *name,
> >  	if (IS_ERR(req))
> >  		return PTR_ERR(req);
> >  
> > -	inode_lock(d_inode(parent));
> > -
> > +	inode_lock(dir);
> >  	req->r_inode = inode;
> >  	ihold(inode);
> >  	req->r_ino2 = ceph_vino(d_inode(parent));
> > -	req->r_parent = d_inode(parent);
> > +	req->r_parent = dir;
> >  	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
> >  	req->r_num_caps = 2;
> >  	err = ceph_mdsc_do_request(mdsc, NULL, req);
> > +	inode_unlock(dir);
> >  
> > -	inode_unlock(d_inode(parent));
> > +	if (err)
> > +		goto out;
> >  
> > -	if (!err) {
> > -		struct ceph_mds_reply_info_parsed *rinfo = &req->r_reply_info;
> > +	rinfo = &req->r_reply_info;
> > +	if (!IS_ENCRYPTED(dir)) {
> >  		memcpy(name, rinfo->dname, rinfo->dname_len);
> >  		name[rinfo->dname_len] = 0;
> > -		dout("get_name %p ino %llx.%llx name %s\n",
> > -		     child, ceph_vinop(inode), name);
> >  	} else {
> > -		dout("get_name %p ino %llx.%llx err %d\n",
> > -		     child, ceph_vinop(inode), err);
> > -	}
> > +		struct fscrypt_str oname = FSTR_INIT(NULL, 0);
> > +		struct ceph_fname fname = { .dir	= dir,
> > +					    .name	= rinfo->dname,
> > +					    .ctext	= rinfo->altname,
> > +					    .name_len	= rinfo->dname_len,
> > +					    .ctext_len	= rinfo->altname_len };
> > +
> > +		err = ceph_fname_alloc_buffer(dir, &oname);
> > +		if (err < 0)
> > +			goto out;
> >  
> > +		err = ceph_fname_to_usr(&fname, NULL, &oname, NULL);
> > +		if (!err) {
> > +			memcpy(name, oname.name, oname.len);
> > +			name[oname.len] = 0;
> > +		}
> > +		ceph_fname_free_buffer(dir, &oname);
> > +	}
> > +out:
> > +	dout("get_name %p ino %llx.%llx err %d %s%s\n",
> > +		     child, ceph_vinop(inode), err,
> > +		     err ? "" : "name ", err ? "" : name);
> >  	ceph_mdsc_put_request(req);
> >  	return err;
> >  }
> > -- 
> > 2.30.2
> > 

-- 
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
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 [this message]
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=8df5d18a65be8385f915dd7f3655db90d905b7c7.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