This patch series prevents inheritance and setting of various flags,
as appropriate for specific inode types. Flags which should be inherited
are listed explicitly so as to prevent future flags being overlooked and
inherited by accident. It introduces a function to mask flags based on
the inode type and uses it in inode creation and the SETFLAGS ioctl to
facilitate future consistency.
This fixes the TOPDIR flag inheritance bug reported at
http://bugzilla.kernel.org/show_bug.cgi?id=9866.
This version introduces the restrictions on setting flags for
inappropriate inodes and lists inheritable flags explicitly.
Cheers,
Duane.
At the moment there are few restrictions on which flags may be set on which
inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
and APPEND may not be set on links. Tighten that to disallow TOPDIR being
set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
NOCOMP, ECOMPR, INDEX, JOURNAL_DATA and NOTAIL being set on anything but
regular files or directories.
Introduce a flags masking function which masks flags based on mode and use
it during inode creation and when flags are set via the ioctl to facilitate
future consistency.
Signed-off-by: Duane Griffin <[email protected]>
--
The specific flags masked out here are those suggested by Andreas, as well
as IMMUTABLE, which I included to match the behaviour of the existing code.
It would be good to get some review of these.
---
fs/ext2/ialloc.c | 8 ++------
fs/ext2/ioctl.c | 3 +--
include/linux/ext2_fs.h | 22 ++++++++++++++++++++++
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index a51d4ca..1559b8e 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -565,12 +565,8 @@ got:
inode->i_blocks = 0;
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
memset(ei->i_data, 0, sizeof(ei->i_data));
- ei->i_flags = EXT2_I(dir)->i_flags & EXT2_FL_INHERITED;
- if (S_ISLNK(mode))
- ei->i_flags &= ~(EXT2_IMMUTABLE_FL|EXT2_APPEND_FL);
- /* dirsync is only applied to directories */
- if (!S_ISDIR(mode))
- ei->i_flags &= ~EXT2_DIRSYNC_FL;
+ ei->i_flags =
+ ext2_mask_flags(mode, EXT2_I(dir)->i_flags & EXT2_FL_INHERITED);
ei->i_faddr = 0;
ei->i_frag_no = 0;
ei->i_frag_size = 0;
diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
index de876fa..7cb4bad 100644
--- a/fs/ext2/ioctl.c
+++ b/fs/ext2/ioctl.c
@@ -50,8 +50,7 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
goto setflags_out;
}
- if (!S_ISDIR(inode->i_mode))
- flags &= ~EXT2_DIRSYNC_FL;
+ flags = ext2_mask_flags(inode->i_mode, flags);
mutex_lock(&inode->i_mutex);
/* Is it quota file? Do not allow user to mess with it */
diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index 7ead7eb..48a6cc1 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -201,6 +201,28 @@ struct ext2_group_desc
EXT2_NOCOMP_FL | EXT2_JOURNAL_DATA_FL |\
EXT2_NOTAIL_FL | EXT2_DIRSYNC_FL)
+/* Flags that are inappropriate for regular files. */
+#define EXT2_REG_FLMASK (EXT2_DIRSYNC_FL | EXT2_TOPDIR_FL)
+
+/* Flags that are inappropriate for non-directories/regular files. */
+#define EXT2_OTHER_FLMASK (EXT2_SECRM_FL | EXT2_UNRM_FL | EXT2_COMPR_FL |\
+ EXT2_SYNC_FL | EXT2_IMMUTABLE_FL | EXT2_APPEND_FL |\
+ EXT2_DIRTY_FL | EXT2_COMPRBLK_FL | EXT2_NOCOMP_FL |\
+ EXT2_ECOMPR_FL | EXT2_INDEX_FL |\
+ EXT2_JOURNAL_DATA_FL | EXT2_NOTAIL_FL |\
+ EXT2_DIRSYNC_FL | EXT2_TOPDIR_FL)
+
+/* Mask out flags that are inappropriate for the given type of inode. */
+static inline __le32 ext2_mask_flags(__le16 mode, __le32 flags)
+{
+ if (S_ISDIR(mode))
+ return flags;
+ else if (S_ISREG(mode))
+ return flags & ~EXT2_REG_FLMASK;
+ else
+ return flags & ~EXT2_OTHER_FLMASK;
+}
+
/*
* ioctl commands
*/
--
1.5.3.7
At the moment there are few restrictions on which flags may be set on which
inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
and APPEND may not be set on links. Tighten that to disallow TOPDIR being
set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
NOCOMPR, ECOMPR, INDEX, JOURNAL_DATA, NOTAIL, HUGE_FILE, EXTENTS or
EXT_MIGRATE being set on anything but regular files or directories.
Introduce a flags masking function which masks flags based on mode and use
it during inode creation and when flags are set via the ioctl to facilitate
future consistency.
Signed-off-by: Duane Griffin <[email protected]>
--
The specific flags masked out here are those suggested by Andreas, as well
as IMMUTABLE, which I included to match the behaviour of the existing code,
and EXT_MIGRATE.
It would be good to get some review of these.
---
fs/ext4/ext4.h | 24 ++++++++++++++++++++++++
fs/ext4/ialloc.c | 14 +++++---------
fs/ext4/ioctl.c | 3 +--
3 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1a4faa5..2aafd1b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -243,6 +243,30 @@ struct ext4_group_desc
EXT4_JOURNAL_DATA_FL | EXT4_NOTAIL_FL|\
EXT4_DIRSYNC_FL)
+/* Flags that are inappropriate for regular files. */
+#define EXT4_REG_FLMASK (EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL)
+
+/* Flags that are inappropriate for non-directories/regular files. */
+#define EXT4_OTHER_FLMASK (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
+ EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
+ EXT4_DIRTY_FL | EXT4_COMPRBLK_FL | EXT4_NOCOMPR_FL|\
+ EXT4_ECOMPR_FL | EXT4_INDEX_FL |\
+ EXT4_JOURNAL_DATA_FL | EXT4_NOTAIL_FL |\
+ EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL |\
+ EXT4_HUGE_FILE_FL | EXT4_EXTENTS_FL |\
+ EXT4_EXT_MIGRATE)
+
+/* Mask out flags that are inappropriate for the given type of inode. */
+static inline __le32 ext4_mask_flags(__le16 mode, __le32 flags)
+{
+ if (S_ISDIR(mode))
+ return flags;
+ else if (S_ISREG(mode))
+ return flags & ~EXT4_REG_FLMASK;
+ else
+ return flags & ~EXT4_OTHER_FLMASK;
+}
+
/*
* Inode dynamic state flags
*/
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index ff25d57..11fb561 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -698,16 +698,12 @@ got:
ei->i_disksize = 0;
/*
- * Don't inherit extent flag from directory. We set extent flag on
- * newly created directory and file only if -o extent mount option is
- * specified
+ * Don't inherit extent flag from directory, amongst others. We set
+ * extent flag on newly created directory and file only if -o extent
+ * mount option is specified
*/
- ei->i_flags = EXT4_I(dir)->i_flags & EXT4_FL_INHERITED;
- if (S_ISLNK(mode))
- ei->i_flags &= ~(EXT4_IMMUTABLE_FL|EXT4_APPEND_FL);
- /* dirsync only applies to directories */
- if (!S_ISDIR(mode))
- ei->i_flags &= ~EXT4_DIRSYNC_FL;
+ ei->i_flags =
+ ext4_mask_flags(mode, EXT4_I(dir)->i_flags & EXT4_FL_INHERITED);
ei->i_file_acl = 0;
ei->i_dtime = 0;
ei->i_block_alloc_info = NULL;
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7a6c2f1..ba0df2b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -49,8 +49,7 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
if (err)
return err;
- if (!S_ISDIR(inode->i_mode))
- flags &= ~EXT4_DIRSYNC_FL;
+ flags = ext4_mask_flags(inode->i_mode, flags);
err = -EPERM;
mutex_lock(&inode->i_mutex);
--
1.5.3.7
At the moment there are few restrictions on which flags may be set on which
inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
and APPEND may not be set on links. Tighten that to disallow TOPDIR being
set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
NOCOMPR, ECOMPR, INDEX, JOURNAL_DATA and NOTAIL being set on anything but
regular files or directories.
Introduce a flags masking function which masks flags based on mode and use
it during inode creation and when flags are set via the ioctl to facilitate
future consistency.
Signed-off-by: Duane Griffin <[email protected]>
--
The specific flags masked out here are those suggested by Andreas, as well
as IMMUTABLE, which I included to match the behaviour of the existing code.
It would be good to get some review of these.
---
fs/ext3/ialloc.c | 8 ++------
fs/ext3/ioctl.c | 3 +--
include/linux/ext3_fs.h | 22 ++++++++++++++++++++++
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index 1d9fe3f..c72d49d 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -559,12 +559,8 @@ got:
ei->i_dir_start_lookup = 0;
ei->i_disksize = 0;
- ei->i_flags = EXT3_I(dir)->i_flags & EXT3_FL_INHERITED;
- if (S_ISLNK(mode))
- ei->i_flags &= ~(EXT3_IMMUTABLE_FL|EXT3_APPEND_FL);
- /* dirsync only applies to directories */
- if (!S_ISDIR(mode))
- ei->i_flags &= ~EXT3_DIRSYNC_FL;
+ ei->i_flags =
+ ext3_mask_flags(mode, EXT3_I(dir)->i_flags & EXT3_FL_INHERITED);
#ifdef EXT3_FRAGMENTS
ei->i_faddr = 0;
ei->i_frag_no = 0;
diff --git a/fs/ext3/ioctl.c b/fs/ext3/ioctl.c
index 0d0c701..6d6534d 100644
--- a/fs/ext3/ioctl.c
+++ b/fs/ext3/ioctl.c
@@ -53,8 +53,7 @@ int ext3_ioctl (struct inode * inode, struct file * filp, unsigned int cmd,
goto flags_out;
}
- if (!S_ISDIR(inode->i_mode))
- flags &= ~EXT3_DIRSYNC_FL;
+ flags = ext3_mask_flags(inode->i_mode, flags);
mutex_lock(&inode->i_mutex);
/* Is it quota file? Do not allow user to mess with it */
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 140190d..c22e1c1 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -185,6 +185,28 @@ struct ext3_group_desc
EXT3_NOCOMPR_FL | EXT3_JOURNAL_DATA_FL |\
EXT3_NOTAIL_FL | EXT3_DIRSYNC_FL)
+/* Flags that are inappropriate for regular files. */
+#define EXT3_REG_FLMASK (EXT3_DIRSYNC_FL | EXT3_TOPDIR_FL)
+
+/* Flags that are inappropriate for non-directories/regular files. */
+#define EXT3_OTHER_FLMASK (EXT3_SECRM_FL | EXT3_UNRM_FL | EXT3_COMPR_FL |\
+ EXT3_SYNC_FL | EXT3_IMMUTABLE_FL | EXT3_APPEND_FL |\
+ EXT3_DIRTY_FL | EXT3_COMPRBLK_FL | EXT3_NOCOMPR_FL|\
+ EXT3_ECOMPR_FL | EXT3_INDEX_FL |\
+ EXT3_JOURNAL_DATA_FL | EXT3_NOTAIL_FL |\
+ EXT3_DIRSYNC_FL | EXT3_TOPDIR_FL)
+
+/* Mask out flags that are inappropriate for the given type of inode. */
+static inline __le32 ext3_mask_flags(__le16 mode, __le32 flags)
+{
+ if (S_ISDIR(mode))
+ return flags;
+ else if (S_ISREG(mode))
+ return flags & ~EXT3_REG_FLMASK;
+ else
+ return flags & ~EXT3_OTHER_FLMASK;
+}
+
/*
* Inode dynamic state flags
*/
--
1.5.3.7
At present INDEX and EXTENTS are the only flags that new ext4 inodes do
NOT inherit from their parent. In addition prevent the flags DIRTY, ECOMPR,
IMAGIC, TOPDIR, HUGE_FILE and EXT_MIGRATE from being inherited. List
inheritable flags explicitly to prevent future flags from accidentally being
inherited.
Signed-off-by: Duane Griffin <[email protected]>
--
This is v2 with inheritable flags now explicitly specified, as per Andreas'
request.
---
fs/ext4/ext4.h | 8 ++++++++
fs/ext4/ialloc.c | 2 +-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8158083..1a4faa5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -235,6 +235,14 @@ struct ext4_group_desc
#define EXT4_FL_USER_VISIBLE 0x000BDFFF /* User visible flags */
#define EXT4_FL_USER_MODIFIABLE 0x000380FF /* User modifiable flags */
+/* Flags that should be inherited by new inodes from their parent. */
+#define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
+ EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
+ EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
+ EXT4_COMPRBLK_FL | EXT4_NOCOMPR_FL |\
+ EXT4_JOURNAL_DATA_FL | EXT4_NOTAIL_FL|\
+ EXT4_DIRSYNC_FL)
+
/*
* Inode dynamic state flags
*/
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index c6efbab..ff25d57 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -702,7 +702,7 @@ got:
* newly created directory and file only if -o extent mount option is
* specified
*/
- ei->i_flags = EXT4_I(dir)->i_flags & ~(EXT4_INDEX_FL|EXT4_EXTENTS_FL);
+ ei->i_flags = EXT4_I(dir)->i_flags & EXT4_FL_INHERITED;
if (S_ISLNK(mode))
ei->i_flags &= ~(EXT4_IMMUTABLE_FL|EXT4_APPEND_FL);
/* dirsync only applies to directories */
--
1.5.3.7
At present BTREE/INDEX is the only flag that new ext2 inodes do NOT
inherit from their parent. In addition prevent the flags DIRTY, ECOMPR,
INDEX, IMAGIC and TOPDIR from being inherited. List inheritable flags
explicitly to prevent future flags from accidentally being inherited.
Signed-off-by: Duane Griffin <[email protected]>
--
This is v2 with inheritable flags now explicitly specified, as per Andreas'
request. The BTREE flag has also been removed as it is just an alias for
INDEX, as also pointed out by Andreas.
---
fs/ext2/ialloc.c | 2 +-
include/linux/ext2_fs.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index f597413..a51d4ca 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -565,7 +565,7 @@ got:
inode->i_blocks = 0;
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
memset(ei->i_data, 0, sizeof(ei->i_data));
- ei->i_flags = EXT2_I(dir)->i_flags & ~EXT2_BTREE_FL;
+ ei->i_flags = EXT2_I(dir)->i_flags & EXT2_FL_INHERITED;
if (S_ISLNK(mode))
ei->i_flags &= ~(EXT2_IMMUTABLE_FL|EXT2_APPEND_FL);
/* dirsync is only applied to directories */
diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index 84cec2a..7ead7eb 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -194,6 +194,13 @@ struct ext2_group_desc
#define EXT2_FL_USER_VISIBLE FS_FL_USER_VISIBLE /* User visible flags */
#define EXT2_FL_USER_MODIFIABLE FS_FL_USER_MODIFIABLE /* User modifiable flags */
+/* Flags that should be inherited by new inodes from their parent. */
+#define EXT2_FL_INHERITED (EXT2_SECRM_FL | EXT2_UNRM_FL | EXT2_COMPR_FL |\
+ EXT2_SYNC_FL | EXT2_IMMUTABLE_FL | EXT2_APPEND_FL |\
+ EXT2_NODUMP_FL | EXT2_NOATIME_FL | EXT2_COMPRBLK_FL|\
+ EXT2_NOCOMP_FL | EXT2_JOURNAL_DATA_FL |\
+ EXT2_NOTAIL_FL | EXT2_DIRSYNC_FL)
+
/*
* ioctl commands
*/
--
1.5.3.7
At present INDEX is the only flag that new ext3 inodes do NOT inherit from
their parent. In addition prevent the flags DIRTY, ECOMPR, IMAGIC and
TOPDIR from being inherited. List inheritable flags explicitly to prevent
future flags from accidentally being inherited.
Signed-off-by: Duane Griffin <[email protected]>
--
This is v2 with inheritable flags now explicitly specified, as per Andreas'
request.
---
fs/ext3/ialloc.c | 2 +-
include/linux/ext3_fs.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index 7712682..1d9fe3f 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -559,7 +559,7 @@ got:
ei->i_dir_start_lookup = 0;
ei->i_disksize = 0;
- ei->i_flags = EXT3_I(dir)->i_flags & ~EXT3_INDEX_FL;
+ ei->i_flags = EXT3_I(dir)->i_flags & EXT3_FL_INHERITED;
if (S_ISLNK(mode))
ei->i_flags &= ~(EXT3_IMMUTABLE_FL|EXT3_APPEND_FL);
/* dirsync only applies to directories */
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 36c5403..140190d 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -178,6 +178,13 @@ struct ext3_group_desc
#define EXT3_FL_USER_VISIBLE 0x0003DFFF /* User visible flags */
#define EXT3_FL_USER_MODIFIABLE 0x000380FF /* User modifiable flags */
+/* Flags that should be inherited by new inodes from their parent. */
+#define EXT3_FL_INHERITED (EXT3_SECRM_FL | EXT3_UNRM_FL | EXT3_COMPR_FL |\
+ EXT3_SYNC_FL | EXT3_IMMUTABLE_FL | EXT3_APPEND_FL |\
+ EXT3_NODUMP_FL | EXT3_NOATIME_FL | EXT3_COMPRBLK_FL|\
+ EXT3_NOCOMPR_FL | EXT3_JOURNAL_DATA_FL |\
+ EXT3_NOTAIL_FL | EXT3_DIRSYNC_FL)
+
/*
* Inode dynamic state flags
*/
--
1.5.3.7
On Wed, Jun 11, 2008 at 01:04:09AM +0100, Duane Griffin wrote:
> At the moment there are few restrictions on which flags may be set on which
> inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
> and APPEND may not be set on links. Tighten that to disallow TOPDIR being
> set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
> NOCOMPR, ECOMPR, INDEX, JOURNAL_DATA, NOTAIL, HUGE_FILE, EXTENTS or
> EXT_MIGRATE being set on anything but regular files or directories.
>
> Introduce a flags masking function which masks flags based on mode and use
> it during inode creation and when flags are set via the ioctl to facilitate
> future consistency.
>
> Signed-off-by: Duane Griffin <[email protected]>
> --
>
> The specific flags masked out here are those suggested by Andreas, as well
> as IMMUTABLE, which I included to match the behaviour of the existing code,
> and EXT_MIGRATE.
>
> It would be good to get some review of these.
>
> ---
> fs/ext4/ext4.h | 24 ++++++++++++++++++++++++
> fs/ext4/ialloc.c | 14 +++++---------
> fs/ext4/ioctl.c | 3 +--
> 3 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1a4faa5..2aafd1b 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -243,6 +243,30 @@ struct ext4_group_desc
> EXT4_JOURNAL_DATA_FL | EXT4_NOTAIL_FL|\
> EXT4_DIRSYNC_FL)
>
> +/* Flags that are inappropriate for regular files. */
> +#define EXT4_REG_FLMASK (EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL)
> +
> +/* Flags that are inappropriate for non-directories/regular files. */
> +#define EXT4_OTHER_FLMASK (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
> + EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
> + EXT4_DIRTY_FL | EXT4_COMPRBLK_FL | EXT4_NOCOMPR_FL|\
> + EXT4_ECOMPR_FL | EXT4_INDEX_FL |\
> + EXT4_JOURNAL_DATA_FL | EXT4_NOTAIL_FL |\
> + EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL |\
> + EXT4_HUGE_FILE_FL | EXT4_EXTENTS_FL |\
> + EXT4_EXT_MIGRATE)
> +
> +/* Mask out flags that are inappropriate for the given type of inode. */
> +static inline __le32 ext4_mask_flags(__le16 mode, __le32 flags)
> +{
> + if (S_ISDIR(mode))
> + return flags;
> + else if (S_ISREG(mode))
> + return flags & ~EXT4_REG_FLMASK;
> + else
> + return flags & ~EXT4_OTHER_FLMASK;
> +}
> +
why are the arguments __le32 ? They should be in host order.
static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
-aneesh
2008/6/11 Aneesh Kumar K.V <[email protected]>:
>> +/* Mask out flags that are inappropriate for the given type of inode. */
>> +static inline __le32 ext4_mask_flags(__le16 mode, __le32 flags)
>> +{
>> + if (S_ISDIR(mode))
>> + return flags;
>> + else if (S_ISREG(mode))
>> + return flags & ~EXT4_REG_FLMASK;
>> + else
>> + return flags & ~EXT4_OTHER_FLMASK;
>> +}
>> +
>
> why are the arguments __le32 ? They should be in host order.
>
> static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
Whoops, I was matching the fields in ex4_inode, but I should have been
looking at ext4_inode_info. Shouldn't sparse have complained about
that?
Thanks,
Duane.
--
"I never could learn to drink that blood and call it wine" - Bob Dylan
On Jun 11, 2008 01:04 +0100, Duane Griffin wrote:
> At the moment there are few restrictions on which flags may be set on which
> inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
> and APPEND may not be set on links. Tighten that to disallow TOPDIR being
> set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
> NOCOMP, ECOMPR, INDEX, JOURNAL_DATA and NOTAIL being set on anything but
> regular files or directories.
>
> Introduce a flags masking function which masks flags based on mode and use
> it during inode creation and when flags are set via the ioctl to facilitate
> future consistency.
This second set of patches is missing out on the presence of the
"EXT2_FL_USER_MODIFIABLE" mask in ext2_ioctl(EXT2_IOC_SETFLAGS). This is
what prevents "unsettable" flags from being set from userspace.
I don't have any objection to additional filtering to avoid setting the
USER_MODIFIABLE flags on special files.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
2008/6/12 Andreas Dilger <[email protected]>:
> On Jun 11, 2008 01:04 +0100, Duane Griffin wrote:
>> At the moment there are few restrictions on which flags may be set on which
>> inodes. Specifically DIRSYNC may only be set on directories and IMMUTABLE
>> and APPEND may not be set on links. Tighten that to disallow TOPDIR being
>> set on non-directories and SECRM, UNRM, COMPR, SYNC, DIRTY, COMPRBLK,
>> NOCOMP, ECOMPR, INDEX, JOURNAL_DATA and NOTAIL being set on anything but
>> regular files or directories.
>>
>> Introduce a flags masking function which masks flags based on mode and use
>> it during inode creation and when flags are set via the ioctl to facilitate
>> future consistency.
>
> This second set of patches is missing out on the presence of the
> "EXT2_FL_USER_MODIFIABLE" mask in ext2_ioctl(EXT2_IOC_SETFLAGS). This is
> what prevents "unsettable" flags from being set from userspace.
>
> I don't have any objection to additional filtering to avoid setting the
> USER_MODIFIABLE flags on special files.
OK, thanks. I convinced myself that the patch wouldn't actually change
behaviour in the ioctl case, but thought it best to use it anyway for
consistency and to avoid future problems. I should probably have
mentioned that in the ChangeLog; I never know quite how exhaustive to
be in these things...
> Cheers, Andreas
Cheers,
Duane.
--
"I never could learn to drink that blood and call it wine" - Bob Dylan
On Wed, 2008-06-11 at 12:49 +0100, Duane Griffin wrote:
> 2008/6/11 Aneesh Kumar K.V <[email protected]>:
> >> +/* Mask out flags that are inappropriate for the given type of inode. */
> >> +static inline __le32 ext4_mask_flags(__le16 mode, __le32 flags)
> >> +{
> >> + if (S_ISDIR(mode))
> >> + return flags;
> >> + else if (S_ISREG(mode))
> >> + return flags & ~EXT4_REG_FLMASK;
> >> + else
> >> + return flags & ~EXT4_OTHER_FLMASK;
> >> +}
> >> +
> >
> > why are the arguments __le32 ? They should be in host order.
> >
> > static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
>
> Whoops, I was matching the fields in ex4_inode, but I should have been
> looking at ext4_inode_info. Shouldn't sparse have complained about
> that?
It does, but not by default. You need to run sparse with
-D__CHECK_ENDIAN__
I use this alias:
make_sparse is aliased to `make C=2 CF="-D__CHECK_ENDIAN__"'
Shaggy
--
David Kleikamp
IBM Linux Technology Center
2008/6/12 Dave Kleikamp <[email protected]>:
>> Whoops, I was matching the fields in ex4_inode, but I should have been
>> looking at ext4_inode_info. Shouldn't sparse have complained about
>> that?
>
> It does, but not by default. You need to run sparse with
> -D__CHECK_ENDIAN__
>
> I use this alias:
> make_sparse is aliased to `make C=2 CF="-D__CHECK_ENDIAN__"'
That does the trick nicely, thanks!
> Shaggy
Cheers,
Duane.
--
"I never could learn to drink that blood and call it wine" - Bob Dylan