Switch ceph atomic open to use fscrypt_prepare_atomic_open(). This fixes
a bug where a dentry is incorrectly set with DCACHE_NOKEY_NAME when 'dir'
has been evicted but the key is still available (for example, where there's
a drop_caches).
Signed-off-by: Luís Henriques <[email protected]>
---
fs/ceph/file.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index dee3b445f415..5ad57cc4c13b 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -795,11 +795,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
ihold(dir);
if (IS_ENCRYPTED(dir)) {
set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
- if (!fscrypt_has_encryption_key(dir)) {
- spin_lock(&dentry->d_lock);
- dentry->d_flags |= DCACHE_NOKEY_NAME;
- spin_unlock(&dentry->d_lock);
- }
+ err = fscrypt_prepare_atomic_open(dir, dentry);
+ if (err)
+ goto out_req;
}
if (flags & O_CREAT) {
On Mon, Mar 13, 2023 at 12:33:10PM +0000, Lu?s Henriques wrote:
> Switch ceph atomic open to use fscrypt_prepare_atomic_open(). This fixes
> a bug where a dentry is incorrectly set with DCACHE_NOKEY_NAME when 'dir'
> has been evicted but the key is still available (for example, where there's
> a drop_caches).
>
> Signed-off-by: Lu?s Henriques <[email protected]>
> ---
> fs/ceph/file.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index dee3b445f415..5ad57cc4c13b 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -795,11 +795,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> ihold(dir);
> if (IS_ENCRYPTED(dir)) {
> set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> - if (!fscrypt_has_encryption_key(dir)) {
> - spin_lock(&dentry->d_lock);
> - dentry->d_flags |= DCACHE_NOKEY_NAME;
> - spin_unlock(&dentry->d_lock);
> - }
> + err = fscrypt_prepare_atomic_open(dir, dentry);
> + if (err)
> + goto out_req;
Note that this patch does not apply to upstream or even to linux-next.
I'd be glad to take patch 1 through the fscrypt tree for 6.4. But I'm wondering
what the current plans are for getting ceph's fscrypt support upstream?
- Eric
Eric Biggers <[email protected]> writes:
> On Mon, Mar 13, 2023 at 12:33:10PM +0000, Luís Henriques wrote:
>> Switch ceph atomic open to use fscrypt_prepare_atomic_open(). This fixes
>> a bug where a dentry is incorrectly set with DCACHE_NOKEY_NAME when 'dir'
>> has been evicted but the key is still available (for example, where there's
>> a drop_caches).
>>
>> Signed-off-by: Luís Henriques <[email protected]>
>> ---
>> fs/ceph/file.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index dee3b445f415..5ad57cc4c13b 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -795,11 +795,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>> ihold(dir);
>> if (IS_ENCRYPTED(dir)) {
>> set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
>> - if (!fscrypt_has_encryption_key(dir)) {
>> - spin_lock(&dentry->d_lock);
>> - dentry->d_flags |= DCACHE_NOKEY_NAME;
>> - spin_unlock(&dentry->d_lock);
>> - }
>> + err = fscrypt_prepare_atomic_open(dir, dentry);
>> + if (err)
>> + goto out_req;
>
> Note that this patch does not apply to upstream or even to linux-next.
True, I should have mentioned that in the cover-letter. This patch should
be applied against the 'testing' branch in https://github.com/ceph/ceph-client,
which is where the ceph fscrypt currently lives.
> I'd be glad to take patch 1 through the fscrypt tree for 6.4. But I'm wondering
> what the current plans are for getting ceph's fscrypt support upstream?
As far as I know, the current plan is to try to merge the ceph code during
the next merge window for 6.4 (but Xiubo and Ilya may correct me if I'm
wrong). Also, regarding who picks which patch, I'm fine with you picking
the first one. But I'll let the ceph maintainers say what they think,
because it may be easier for them to keep both patches together due to the
testing infrastructure being used.
Anyway, I'll send out a new rev tomorrow taking your comments into
account. Thanks, Eric!
Cheers,
--
Luís
On 14/03/2023 02:42, Luís Henriques wrote:
> Eric Biggers <[email protected]> writes:
>
>> On Mon, Mar 13, 2023 at 12:33:10PM +0000, Luís Henriques wrote:
>>> Switch ceph atomic open to use fscrypt_prepare_atomic_open(). This fixes
>>> a bug where a dentry is incorrectly set with DCACHE_NOKEY_NAME when 'dir'
>>> has been evicted but the key is still available (for example, where there's
>>> a drop_caches).
>>>
>>> Signed-off-by: Luís Henriques <[email protected]>
>>> ---
>>> fs/ceph/file.c | 8 +++-----
>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>>> index dee3b445f415..5ad57cc4c13b 100644
>>> --- a/fs/ceph/file.c
>>> +++ b/fs/ceph/file.c
>>> @@ -795,11 +795,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>>> ihold(dir);
>>> if (IS_ENCRYPTED(dir)) {
>>> set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
>>> - if (!fscrypt_has_encryption_key(dir)) {
>>> - spin_lock(&dentry->d_lock);
>>> - dentry->d_flags |= DCACHE_NOKEY_NAME;
>>> - spin_unlock(&dentry->d_lock);
>>> - }
>>> + err = fscrypt_prepare_atomic_open(dir, dentry);
>>> + if (err)
>>> + goto out_req;
>> Note that this patch does not apply to upstream or even to linux-next.
> True, I should have mentioned that in the cover-letter. This patch should
> be applied against the 'testing' branch in https://github.com/ceph/ceph-client,
> which is where the ceph fscrypt currently lives.
>
>> I'd be glad to take patch 1 through the fscrypt tree for 6.4. But I'm wondering
>> what the current plans are for getting ceph's fscrypt support upstream?
> As far as I know, the current plan is to try to merge the ceph code during
> the next merge window for 6.4 (but Xiubo and Ilya may correct me if I'm
> wrong). Also, regarding who picks which patch, I'm fine with you picking
> the first one. But I'll let the ceph maintainers say what they think,
> because it may be easier for them to keep both patches together due to the
> testing infrastructure being used.
>
> Anyway, I'll send out a new rev tomorrow taking your comments into
> account. Thanks, Eric!
Eric, Luis,
It will be fine if Eric could merge patch 1 into the fscrypt tree. Then
I will merge the patch 1 into the ceph-client's testing by tagging as
[DO NOT MERGE] to run our tests.
And locally we are still running the test, and there have several fixes
followed and need more time to review.
Thanks
- Xiubo
> Cheers,
--
Best Regards,
Xiubo Li (李秀波)
Email: [email protected]/[email protected]
Slack: @Xiubo Li
Xiubo Li <[email protected]> writes:
> On 14/03/2023 02:42, Luís Henriques wrote:
>> Eric Biggers <[email protected]> writes:
>>
>>> On Mon, Mar 13, 2023 at 12:33:10PM +0000, Luís Henriques wrote:
>>>> Switch ceph atomic open to use fscrypt_prepare_atomic_open(). This fixes
>>>> a bug where a dentry is incorrectly set with DCACHE_NOKEY_NAME when 'dir'
>>>> has been evicted but the key is still available (for example, where there's
>>>> a drop_caches).
>>>>
>>>> Signed-off-by: Luís Henriques <[email protected]>
>>>> ---
>>>> fs/ceph/file.c | 8 +++-----
>>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>>>> index dee3b445f415..5ad57cc4c13b 100644
>>>> --- a/fs/ceph/file.c
>>>> +++ b/fs/ceph/file.c
>>>> @@ -795,11 +795,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>>>> ihold(dir);
>>>> if (IS_ENCRYPTED(dir)) {
>>>> set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
>>>> - if (!fscrypt_has_encryption_key(dir)) {
>>>> - spin_lock(&dentry->d_lock);
>>>> - dentry->d_flags |= DCACHE_NOKEY_NAME;
>>>> - spin_unlock(&dentry->d_lock);
>>>> - }
>>>> + err = fscrypt_prepare_atomic_open(dir, dentry);
>>>> + if (err)
>>>> + goto out_req;
>>> Note that this patch does not apply to upstream or even to linux-next.
>> True, I should have mentioned that in the cover-letter. This patch should
>> be applied against the 'testing' branch in https://github.com/ceph/ceph-client,
>> which is where the ceph fscrypt currently lives.
>>
>>> I'd be glad to take patch 1 through the fscrypt tree for 6.4. But I'm wondering
>>> what the current plans are for getting ceph's fscrypt support upstream?
>> As far as I know, the current plan is to try to merge the ceph code during
>> the next merge window for 6.4 (but Xiubo and Ilya may correct me if I'm
>> wrong). Also, regarding who picks which patch, I'm fine with you picking
>> the first one. But I'll let the ceph maintainers say what they think,
>> because it may be easier for them to keep both patches together due to the
>> testing infrastructure being used.
>>
>> Anyway, I'll send out a new rev tomorrow taking your comments into
>> account. Thanks, Eric!
>
> Eric, Luis,
>
> It will be fine if Eric could merge patch 1 into the fscrypt tree. Then I will
> merge the patch 1 into the ceph-client's testing by tagging as [DO NOT MERGE] to
> run our tests.
Awesome, so Eric can pick the first patch. Thanks.
Cheers,
--
Luís
> And locally we are still running the test, and there have several fixes followed
> and need more time to review.
>
> Thanks
>
> - Xiubo
>
>> Cheers,
>
> --
> Best Regards,
>
> Xiubo Li (李秀波)
>
> Email: [email protected]/[email protected]
> Slack: @Xiubo Li
>