From: "Luís Henriques" <lhenriques@suse.de>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Xiubo Li <xiubli@redhat.com>, Jeff Layton <jlayton@kernel.org>,
"Theodore Y. Ts'o" <tytso@mit.edu>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Ilya Dryomov <idryomov@gmail.com>,
linux-fscrypt@vger.kernel.org, ceph-devel@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] fscrypt: new helper function - fscrypt_prepare_atomic_open()
Date: Wed, 15 Mar 2023 17:59:13 +0000 [thread overview]
Message-ID: <873565sxou.fsf@suse.de> (raw)
In-Reply-To: <20230315171249.GA975@sol.localdomain> (Eric Biggers's message of "Wed, 15 Mar 2023 10:12:49 -0700")
Eric Biggers <ebiggers@kernel.org> writes:
> On Wed, Mar 15, 2023 at 11:08:23AM +0000, Luís Henriques wrote:
>> > So, actually I think this patch doesn't make sense. If ceph is doing the above
>> > in its ->lookup() anyway, then it just should do the exact same thing in its
>> > ->atomic_open() too.
>>
>> In fact, my initial fix for the cephfs bug was doing just that. It was a
>> single patch to ceph_atomic_open() that would simply do:
>>
>> if (IS_ENCRYPTED(dir)) {
>> set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
>> err = __fscrypt_prepare_readdir(dir);
>> if (!err && !fscrypt_has_encryption_key(dir)) {
>> spin_lock(&dentry->d_lock);
>> dentry->d_flags |= DCACHE_NOKEY_NAME;
>> spin_unlock(&dentry->d_lock);
>> }
>> }
>>
>> What made me want to create a new helper was that I simply needed to call
>> fscrypt_get_encryption_info() to force the encryption info to be set in
>> the parent directory. But this function was only accessible through
>> __fscrypt_prepare_readdir(), which isn't really a great function name for
>> what I need here.
>>
>> Since __fscrypt_prepare_readdir() doesn't seem to be used anywhere else,
>> maybe it could be removed and fscrypt_get_encryption_info() be exported
>> instead?
>
> Well, fscrypt_get_encryption_info() *used* to be exported, but it was hard to
> keep track of its use cases (some of which were not actually necessary), which
> is why it eventually got replaced with use-case oriented helper functions.
>
> Maybe just use fscrypt_prepare_lookup_partial() for the name of your new helper
> function (instead of fscrypt_prepare_atomic_open())?
OK, thanks for the name suggestion (naming is *indeed* hard). I'll go try
to get a new helper that can be used in both open_atomic and lookup.
That'll require a bit more of testing so that I don't end up breaking
something else.
Cheers,
--
Luís
next prev parent reply other threads:[~2023-03-15 17:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-13 12:33 [PATCH 0/2] ceph: fscrypt: fix atomic open bug for encrypted directories Luís Henriques
2023-03-13 12:33 ` [PATCH 1/2] fscrypt: new helper function - fscrypt_prepare_atomic_open() Luís Henriques
2023-03-13 18:09 ` Eric Biggers
2023-03-14 0:53 ` Xiubo Li
2023-03-14 2:25 ` Eric Biggers
2023-03-14 4:20 ` Xiubo Li
2023-03-14 9:25 ` Luís Henriques
2023-03-14 10:15 ` Luís Henriques
2023-03-14 17:56 ` Eric Biggers
2023-03-15 11:08 ` Luís Henriques
2023-03-15 17:12 ` Eric Biggers
2023-03-15 17:59 ` Luís Henriques [this message]
2023-03-13 12:33 ` [PATCH 2/2] ceph: switch atomic open to use new fscrypt helper Luís Henriques
2023-03-13 18:11 ` Eric Biggers
2023-03-13 18:42 ` Luís Henriques
2023-03-14 0:38 ` Xiubo Li
2023-03-14 9:27 ` Luís Henriques
2023-03-13 17:11 ` [PATCH 0/2] ceph: fscrypt: fix atomic open bug for encrypted directories 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=873565sxou.fsf@suse.de \
--to=lhenriques@suse.de \
--cc=ceph-devel@vger.kernel.org \
--cc=ebiggers@kernel.org \
--cc=idryomov@gmail.com \
--cc=jaegeuk@kernel.org \
--cc=jlayton@kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=xiubli@redhat.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.