Re: [RFC] odd check in ceph_encode_encrypted_dname()

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2025-02-14 at 03:28 +0000, Al Viro wrote:
> On Fri, Feb 14, 2025 at 02:47:56AM +0000, Al Viro wrote:
> 
> [snip]
> 
> > Am I missing something subtle here?  Can elen be non-positive at that point?
> 
> Another fun question: for dentries with name of form _<something>_<inumber>
> we end up looking at fscrypt_has_encryption_key() not for the parent,
> but for inode with inumber encoded in dentry name.  Fair enough, but...
> what happens if we run into such dentry in ceph_mdsc_build_path()?
> 
> There the call of ceph_encode_encrypted_fname() is under
> 	if (fscrypt_has_encryption_key(d_inode(parent)))
> 
> Do we need the keys for both?
> 

That sounds like a bug, but I don't fully recall whether snapshots have
a special case here for some reason. Let me rephrase Al's question:

If I have a snapshot dir that is prefixed with '_', why does it use a
different filename encryption key than other snapshot dirs that don't
start with that character?

My guess here is that this code ought not overwrite "dir" with the
result of parse_longname(), but I don't recall the significance of a
snapshot name that starts with '_'.

        /* Handle the special case of snapshot names that start with '_' */
        if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
            (iname.name[0] == '_')) {
                dir = parse_longname(parent, iname.name, &name_len);
                if (IS_ERR(dir))
                        return PTR_ERR(dir);
                iname.name++; /* skip initial '_' */
        }
        iname.len = name_len;

        if (!fscrypt_has_encryption_key(dir)) {
                memcpy(buf, d_name->name, d_name->len);
                elen = d_name->len;
                goto out;
        }


-- 
Jeff Layton <jlayton@kernel.org>