From: Wang Shilong <[email protected]>
It doesn't make any sense to have project inherit bits
for regular files, even though this won't cause any
problem, but it is better fix this.
Cc: Andreas Dilger <[email protected]>
Signed-off-by: Wang Shilong <[email protected]>
---
fs/ext4/ext4.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1cb67859e051..ceb74093e138 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -421,7 +421,8 @@ struct flex_groups {
EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
/* Flags that are appropriate for regular files (all but dir-specific ones). */
-#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
+#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
+ EXT4_PROJINHERIT_FL))
/* Flags that are appropriate for non-directories/regular files. */
#define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
--
2.21.0
From: Wang Shilong <[email protected]>
It doesn't make any sense to have project inherit bits
for regular files, even though this won't cause any
problem, but it is better fix this.
Cc: Andreas Dilger <[email protected]>
Signed-off-by: Wang Shilong <[email protected]>
---
fs/f2fs/f2fs.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 06b89a9862ab..f02ebecb68ea 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
F2FS_PROJINHERIT_FL)
/* Flags that are appropriate for regular files (all but dir-specific ones). */
-#define F2FS_REG_FLMASK (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
+#define F2FS_REG_FLMASK (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
+ F2FS_PROJINHERIT_FL))
/* Flags that are appropriate for non-directories/regular files. */
#define F2FS_OTHER_FLMASK (F2FS_NODUMP_FL | F2FS_NOATIME_FL)
--
2.21.0
On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <[email protected]>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
It's good to be maintaining consistent behavior with XFS.
Reviewed-by: Darrick J. Wong <[email protected]>
(applies to both ext4 & f2fs patches)
--D
> ---
> fs/ext4/ext4.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
> EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> + EXT4_PROJINHERIT_FL))
>
> /* Flags that are appropriate for non-directories/regular files. */
> #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> --
> 2.21.0
>
On Jun 5, 2019, at 10:32 PM, Wang Shilong <[email protected]> wrote:
>
> From: Wang Shilong <[email protected]>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
> ---
> fs/ext4/ext4.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
> EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> + EXT4_PROJINHERIT_FL))
>
> /* Flags that are appropriate for non-directories/regular files. */
> #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> --
> 2.21.0
>
Cheers, Andreas
On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <[email protected]>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
> ---
> fs/ext4/ext4.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
> EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> + EXT4_PROJINHERIT_FL))
>
> /* Flags that are appropriate for non-directories/regular files. */
> #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> --
> 2.21.0
Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:
if (ext4_mask_flags(inode->i_mode, flags) != flags)
return -EOPNOTSUPP;
- Eric
Hi,
> --
> 2.21.0
Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:
if (ext4_mask_flags(inode->i_mode, flags) != flags)
return -EOPNOTSUPP;
>>>>
You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
this is a bit strange behavior as chattr read existed flags
but could not set them again, there are several possible ways that I could think
of to fix the issue?
1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
2) we automatically fixed the flag before mask check, something like:
if reg:
flags &= ~PROJECT_INHERT;
if (ext4_mask_flags(inode->i_mode, flags) != flags)
return -EOPNOTSUPP;
But this might be not good..
I would prefer solution 1)
What do you think?
On Fri, Jun 07, 2019 at 07:51:18AM +0000, Wang Shilong wrote:
> Hi,
>
> > --
> > 2.21.0
>
> Won't this break 'chattr' on files that already have this flag set?
> FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
> FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:
>
> ? ? ? ? if (ext4_mask_flags(inode->i_mode, flags) != flags)
> ? ? ? ? ? ? ? ? return -EOPNOTSUPP;
>
> >>>>
>
> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
>
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
>
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
> flags &= ~PROJECT_INHERT;
> if (ext4_mask_flags(inode->i_mode, flags) != flags)
> ? ? ? ? ? ? ? ? return -EOPNOTSUPP;
> But this might be not good..
>
> I would prefer solution 1)
> What do you think?
Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time. So (1) wouldn't really work.
A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.
- Eric
Hi,
> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
>
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
>
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
> flags &= ~PROJECT_INHERT;
> if (ext4_mask_flags(inode->i_mode, flags) != flags)
> return -EOPNOTSUPP;
> But this might be not good..
>
> I would prefer solution 1)
> What do you think?
Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time. So (1) wouldn't really work.
A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.
- Eric
>>>>>>
How about fix it in __ext4_iget():
ei->i_flags = le32_to_cpu(raw_inode->i_flags);
if (S_ISREG(inode->i_mode))
ei->i_flags &= ~EXT4_PROJINHERIT_FL;
This way will give a big chance flag will be automatically fixed
next time whenever inode is dirtied.
Thanks,
Shilong
On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <[email protected]>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
Thanks, applied to the ext4 tree.
- Ted
On Fri, Jun 07, 2019 at 11:14:52AM -0700, Eric Biggers wrote:
>
> Existing versions of chattr can't be changed, and people don't necessarily
> upgrade the kernel and e2fsprogs at the same time. So (1) wouldn't really work.
>
> A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
> return the project inherit flag on regular files.
I've amended this patch by adding the following to fix it for
FS_IOC_GETFLAGS (which is chattr uses):
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7af835ac8d23..74648d42c69b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -779,6 +779,8 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return ext4_ioc_getfsmap(sb, (void __user *)arg);
case EXT4_IOC_GETFLAGS:
flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
+ if (S_ISREG(inode->i_mode))
+ flags &= ~EXT4_PROJINHERIT_FL;
return put_user(flags, (int __user *) arg);
case EXT4_IOC_SETFLAGS: {
int err;
- Ted
On 2019/6/6 12:32, Wang Shilong wrote:
> From: Wang Shilong <[email protected]>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
> ---
> fs/f2fs/f2fs.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab..f02ebecb68ea 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
> F2FS_PROJINHERIT_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define F2FS_REG_FLMASK (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
> +#define F2FS_REG_FLMASK (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
> + F2FS_PROJINHERIT_FL))
Hi Shilong,
Could you please add below diff as ext4 did?
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index efdafa886510..295ca5ed42d9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,9 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned
long arg)
fsflags &= F2FS_GETTABLE_FS_FL;
+ if (S_ISREG(inode->i_mode))
+ fsflags &= ~FS_PROJINHERIT_FL;
+
return put_user(fsflags, (int __user *)arg);
}
Thanks,
>
> /* Flags that are appropriate for non-directories/regular files. */
> #define F2FS_OTHER_FLMASK (F2FS_NODUMP_FL | F2FS_NOATIME_FL)
>