mbox series

[f2fs-dev,v7,00/10] Set casefold/fscrypt dentry operations through sb->s_d_op

Message ID 20240221171412.10710-1-krisman@suse.de (mailing list archive)
Headers show
Series Set casefold/fscrypt dentry operations through sb->s_d_op | expand

Message

Gabriel Krisman Bertazi Feb. 21, 2024, 5:14 p.m. UTC
Hi,

v7 of this patchset applying the comments from Eric. Thank you for your
feedback.  Details in changelog of individual patches.

As usual, this survived fstests on ext4 and f2fs.

Thank you,

---
original cover letter:

When case-insensitive and fscrypt were adapted to work together, we moved the
code that sets the dentry operations for case-insensitive dentries(d_hash and
d_compare) to happen from a helper inside ->lookup.  This is because fscrypt
wants to set d_revalidate only on some dentries, so it does it only for them in
d_revalidate.

But, case-insensitive hooks are actually set on all dentries in the filesystem,
so the natural place to do it is through s_d_op and let d_alloc handle it [1].
In addition, doing it inside the ->lookup is a problem for case-insensitive
dentries that are not created through ->lookup, like those coming
open-by-fhandle[2], which will not see the required d_ops.

This patchset therefore reverts to using sb->s_d_op to set the dentry operations
for case-insensitive filesystems.  In order to set case-insensitive hooks early
and not require every dentry to have d_revalidate in case-insensitive
filesystems, it introduces a patch suggested by Al Viro to disable d_revalidate
on some dentries on the fly.

It survives fstests encrypt and quick groups without regressions.  Based on
v6.7-rc1.

[1] https://lore.kernel.org/linux-fsdevel/20231123195327.GP38156@ZenIV/
[2] https://lore.kernel.org/linux-fsdevel/20231123171255.GN38156@ZenIV/

Gabriel Krisman Bertazi (10):
  ovl: Always reject mounting over case-insensitive directories
  fscrypt: Factor out a helper to configure the lookup dentry
  fscrypt: Drop d_revalidate for valid dentries during lookup
  fscrypt: Drop d_revalidate once the key is added
  libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
  libfs: Add helper to choose dentry operations at mount-time
  ext4: Configure dentry operations at dentry-creation time
  f2fs: Configure dentry operations at dentry-creation time
  ubifs: Configure dentry operations at dentry-creation time
  libfs: Drop generic_set_encrypted_ci_d_ops

 fs/crypto/hooks.c       | 15 ++++------
 fs/ext4/namei.c         |  1 -
 fs/ext4/super.c         |  1 +
 fs/f2fs/namei.c         |  1 -
 fs/f2fs/super.c         |  1 +
 fs/libfs.c              | 62 +++++++++++---------------------------
 fs/overlayfs/params.c   | 14 +++++++--
 fs/ubifs/dir.c          |  1 -
 fs/ubifs/super.c        |  1 +
 include/linux/fs.h      | 11 ++++++-
 include/linux/fscrypt.h | 66 ++++++++++++++++++++++++++++++++++++-----
 11 files changed, 105 insertions(+), 69 deletions(-)

Comments

Eric Biggers Feb. 27, 2024, 6:36 a.m. UTC | #1
On Wed, Feb 21, 2024 at 12:14:02PM -0500, Gabriel Krisman Bertazi wrote:
> 
> When case-insensitive and fscrypt were adapted to work together, we moved the
> code that sets the dentry operations for case-insensitive dentries(d_hash and
> d_compare) to happen from a helper inside ->lookup.  This is because fscrypt
> wants to set d_revalidate only on some dentries, so it does it only for them in
> d_revalidate.
> 
> But, case-insensitive hooks are actually set on all dentries in the filesystem,
> so the natural place to do it is through s_d_op and let d_alloc handle it [1].
> In addition, doing it inside the ->lookup is a problem for case-insensitive
> dentries that are not created through ->lookup, like those coming
> open-by-fhandle[2], which will not see the required d_ops.
> 
> This patchset therefore reverts to using sb->s_d_op to set the dentry operations
> for case-insensitive filesystems.  In order to set case-insensitive hooks early
> and not require every dentry to have d_revalidate in case-insensitive
> filesystems, it introduces a patch suggested by Al Viro to disable d_revalidate
> on some dentries on the fly.
> 
> It survives fstests encrypt and quick groups without regressions.  Based on
> v6.7-rc1.
> 
> [1] https://lore.kernel.org/linux-fsdevel/20231123195327.GP38156@ZenIV/
> [2] https://lore.kernel.org/linux-fsdevel/20231123171255.GN38156@ZenIV/
> 
> Gabriel Krisman Bertazi (10):
>   ovl: Always reject mounting over case-insensitive directories
>   fscrypt: Factor out a helper to configure the lookup dentry
>   fscrypt: Drop d_revalidate for valid dentries during lookup
>   fscrypt: Drop d_revalidate once the key is added
>   libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
>   libfs: Add helper to choose dentry operations at mount-time
>   ext4: Configure dentry operations at dentry-creation time
>   f2fs: Configure dentry operations at dentry-creation time
>   ubifs: Configure dentry operations at dentry-creation time
>   libfs: Drop generic_set_encrypted_ci_d_ops
> 
>  fs/crypto/hooks.c       | 15 ++++------
>  fs/ext4/namei.c         |  1 -
>  fs/ext4/super.c         |  1 +
>  fs/f2fs/namei.c         |  1 -
>  fs/f2fs/super.c         |  1 +
>  fs/libfs.c              | 62 +++++++++++---------------------------
>  fs/overlayfs/params.c   | 14 +++++++--
>  fs/ubifs/dir.c          |  1 -
>  fs/ubifs/super.c        |  1 +
>  include/linux/fs.h      | 11 ++++++-
>  include/linux/fscrypt.h | 66 ++++++++++++++++++++++++++++++++++++-----
>  11 files changed, 105 insertions(+), 69 deletions(-)
> 

Looks good,

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric
Gabriel Krisman Bertazi Feb. 27, 2024, 9:28 p.m. UTC | #2
Eric Biggers <ebiggers@kernel.org> writes:

> On Wed, Feb 21, 2024 at 12:14:02PM -0500, Gabriel Krisman Bertazi wrote:
>> 
>> When case-insensitive and fscrypt were adapted to work together, we moved the
>> code that sets the dentry operations for case-insensitive dentries(d_hash and
>> d_compare) to happen from a helper inside ->lookup.  This is because fscrypt
>> wants to set d_revalidate only on some dentries, so it does it only for them in
>> d_revalidate.
>> 
>> But, case-insensitive hooks are actually set on all dentries in the filesystem,
>> so the natural place to do it is through s_d_op and let d_alloc handle it [1].
>> In addition, doing it inside the ->lookup is a problem for case-insensitive
>> dentries that are not created through ->lookup, like those coming
>> open-by-fhandle[2], which will not see the required d_ops.
>> 
>> This patchset therefore reverts to using sb->s_d_op to set the dentry operations
>> for case-insensitive filesystems.  In order to set case-insensitive hooks early
>> and not require every dentry to have d_revalidate in case-insensitive
>> filesystems, it introduces a patch suggested by Al Viro to disable d_revalidate
>> on some dentries on the fly.
>> 
>> It survives fstests encrypt and quick groups without regressions.  Based on
>> v6.7-rc1.
>> 
>> [1] https://lore.kernel.org/linux-fsdevel/20231123195327.GP38156@ZenIV/
>> [2] https://lore.kernel.org/linux-fsdevel/20231123171255.GN38156@ZenIV/
>> 
>> Gabriel Krisman Bertazi (10):
>>   ovl: Always reject mounting over case-insensitive directories
>>   fscrypt: Factor out a helper to configure the lookup dentry
>>   fscrypt: Drop d_revalidate for valid dentries during lookup
>>   fscrypt: Drop d_revalidate once the key is added
>>   libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
>>   libfs: Add helper to choose dentry operations at mount-time
>>   ext4: Configure dentry operations at dentry-creation time
>>   f2fs: Configure dentry operations at dentry-creation time
>>   ubifs: Configure dentry operations at dentry-creation time
>>   libfs: Drop generic_set_encrypted_ci_d_ops
>> 
>>  fs/crypto/hooks.c       | 15 ++++------
>>  fs/ext4/namei.c         |  1 -
>>  fs/ext4/super.c         |  1 +
>>  fs/f2fs/namei.c         |  1 -
>>  fs/f2fs/super.c         |  1 +
>>  fs/libfs.c              | 62 +++++++++++---------------------------
>>  fs/overlayfs/params.c   | 14 +++++++--
>>  fs/ubifs/dir.c          |  1 -
>>  fs/ubifs/super.c        |  1 +
>>  include/linux/fs.h      | 11 ++++++-
>>  include/linux/fscrypt.h | 66 ++++++++++++++++++++++++++++++++++++-----
>>  11 files changed, 105 insertions(+), 69 deletions(-)
>> 
>
> Looks good,
>
> Reviewed-by: Eric Biggers <ebiggers@google.com>

Thank you for you reviews, Eric. I really appreciate them.

Since this been on the list for a while, I pushed it to get some
linux-next testing and, should nothing arise or no one else comments,
will get to Linus soon.

Thanks,
patchwork-bot+f2fs@kernel.org March 18, 2024, 8:45 p.m. UTC | #3
Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Gabriel Krisman Bertazi <krisman@suse.de>:

On Wed, 21 Feb 2024 12:14:02 -0500 you wrote:
> Hi,
> 
> v7 of this patchset applying the comments from Eric. Thank you for your
> feedback.  Details in changelog of individual patches.
> 
> As usual, this survived fstests on ext4 and f2fs.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v7,01/10] ovl: Always reject mounting over case-insensitive directories
    https://git.kernel.org/jaegeuk/f2fs/c/2824083db76c
  - [f2fs-dev,v7,02/10] fscrypt: Factor out a helper to configure the lookup dentry
    https://git.kernel.org/jaegeuk/f2fs/c/8b6bb995d381
  - [f2fs-dev,v7,03/10] fscrypt: Drop d_revalidate for valid dentries during lookup
    https://git.kernel.org/jaegeuk/f2fs/c/e86e6638d117
  - [f2fs-dev,v7,04/10] fscrypt: Drop d_revalidate once the key is added
    https://git.kernel.org/jaegeuk/f2fs/c/e9b10713e82c
  - [f2fs-dev,v7,05/10] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
    https://git.kernel.org/jaegeuk/f2fs/c/e6ca2883d987
  - [f2fs-dev,v7,06/10] libfs: Add helper to choose dentry operations at mount-time
    https://git.kernel.org/jaegeuk/f2fs/c/70dfe3f0d239
  - [f2fs-dev,v7,07/10] ext4: Configure dentry operations at dentry-creation time
    https://git.kernel.org/jaegeuk/f2fs/c/04aa5f4eba49
  - [f2fs-dev,v7,08/10] f2fs: Configure dentry operations at dentry-creation time
    https://git.kernel.org/jaegeuk/f2fs/c/be2760a703e6
  - [f2fs-dev,v7,09/10] ubifs: Configure dentry operations at dentry-creation time
    https://git.kernel.org/jaegeuk/f2fs/c/bc401c2900c1
  - [f2fs-dev,v7,10/10] libfs: Drop generic_set_encrypted_ci_d_ops
    https://git.kernel.org/jaegeuk/f2fs/c/101c3fad29d7

You are awesome, thank you!