| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 11 Sep 2014 01:34:34 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Jan Kara <jack@...e.cz>
Cc: Li Xi <pkuelelixi@...il.com>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
Ext4 Developers List <linux-ext4@...r.kernel.org>,
Theodore Ts'o <tytso@....edu>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"hch@...radead.org" <hch@...radead.org>,
Dmitry Monakhov <dmonakhov@...nvz.org>
Subject: Re: [PATCH v3 2/4] quota: add project quota support
On Sep 10, 2014, at 1:47 PM, Jan Kara <jack@...e.cz> wrote:
> On Wed 10-09-14 11:54:07, Li Xi wrote:
>> Adds project ID support for ext4
>>
>> This patch adds a new internal field of ext4 inode to save project
>> identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
>> inheriting project ID from parent directory.
>>
>> Signed-off-by: Li Xi <lixi <at> ddn.com>
>> ---
>> Index: linux.git/fs/ext4/ialloc.c
>> ===================================================================
>> --- linux.git.orig/fs/ext4/ialloc.c
>> +++ linux.git/fs/ext4/ialloc.c
>> @@ -756,6 +756,16 @@ struct inode *__ext4_new_inode(handle_t
>> inode->i_gid = dir->i_gid;
>> } else
>> inode_init_owner(inode, dir, mode);
>> +#ifdef CONFIG_QUOTA_PROJECT
>> + if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>> + if (ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT)) {
>> + ei->i_projid = EXT4_I(dir)->i_projid;
>> + } else {
>> + ei->i_projid =
>> + make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
>> + }
>> + }
>> +#endif
> Hum, I'd just always store the default project ID in the inode (i.e., you
> can remove the feature test). Then ext4_get_projid() also doesn't have to
> bother checking the feature and can just return whatever is in i_projid.
Makes sense.
>> dquot_initialize(inode);
>>
>> if (!goal)
>> Index: linux.git/fs/ext4/ext4.h
>> ===================================================================
>> --- linux.git.orig/fs/ext4/ext4.h
>> +++ linux.git/fs/ext4/ext4.h
>> @@ -386,16 +386,18 @@ struct flex_groups {
>> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
>> #define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
>> #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
>> +#define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> Why not use the next available flag - 0x00800000?
Because Btrfs is using 0x00800000 for FS_NOCOW_FL, and until such a
time that we need to implement conflicting flags it would be better
to keep them non-overlapping. It seems reasonable that since both
XFS and ext4 have a "PROJINHERIT" flag that this should also be
defined as FS_PROJINHERIT_FL to avoid other conflicts.
Please see lib/ext2fs/ext2_fs.h in newer e2fsprogs.
>> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>>
>> -#define EXT4_FL_USER_VISIBLE 0x004BDFFF /* User visible flags */
>> -#define EXT4_FL_USER_MODIFIABLE 0x004380FF /* User modifiable flags */
>> +#define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
>> +#define EXT4_FL_USER_MODIFIABLE 0x204380FF /* 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_NODUMP_FL | EXT4_NOATIME_FL |\
>> EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
>> - EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
>> + EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL |\
>> + EXT4_PROJINHERIT_FL)
>>
>> /* Flags that are appropriate for regular files (all but dir-specific ones). */
>> #define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
>> @@ -443,6 +445,7 @@ enum {
>> EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
>> EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
>> EXT4_INODE_INLINE_DATA = 28, /* Data in inode. */
>> + EXT4_INODE_PROJINHERIT = 29, /* Create with parents projid */
> This will need updating as well.
Or not...
>> EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
>> };
>>
>> @@ -695,6 +698,7 @@ struct ext4_inode {
>> __le32 i_crtime; /* File Creation time */
>> __le32 i_crtime_extra; /* extra FileCreationtime (nsec << 2 | epoch) */
>> __le32 i_version_hi; /* high 32 bits for 64-bit version */
>> + __le32 i_projid; /* Project ID */
>> };
>>
>> struct move_extent {
>> @@ -943,6 +947,9 @@ struct ext4_inode_info {
>>
>> /* Precomputed uuid+inum+igen checksum for seeding inode checksums */
>> __u32 i_csum_seed;
>> +#ifdef CONFIG_QUOTA_PROJECT
>> + kprojid_t i_projid;
>> +#endif
>> };
>>
>> /*
>> @@ -1525,6 +1532,7 @@ static inline void ext4_clear_state_flag
>> * GDT_CSUM bits are mutually exclusive.
>> */
>> #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
>> +#define EXT4_FEATURE_RO_COMPAT_PROJECT 0x1000 /* Project quota */
> Why not use the next bit available - i.e. 0x0800?
That is also already in use in e2fsprogs:
#define EXT4_FEATURE_RO_COMPAT_REPLICA 0x0800
>> #define EXT4_FEATURE_INCOMPAT_COMPRESSION 0x0001
>> #define EXT4_FEATURE_INCOMPAT_FILETYPE 0x0002
>> @@ -1574,7 +1582,8 @@ static inline void ext4_clear_state_flag
>> EXT4_FEATURE_RO_COMPAT_HUGE_FILE |\
>> EXT4_FEATURE_RO_COMPAT_BIGALLOC |\
>> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM|\
>> - EXT4_FEATURE_RO_COMPAT_QUOTA)
>> + EXT4_FEATURE_RO_COMPAT_QUOTA |\
>> + EXT4_FEATURE_RO_COMPAT_PROJECT)
>>
>> /*
>> * Default values for user and/or group using reserved blocks
>> @@ -1582,6 +1591,11 @@ static inline void ext4_clear_state_flag
>> #define EXT4_DEF_RESUID 0
>> #define EXT4_DEF_RESGID 0
>>
>> +/*
>> + * Default project ID
>> + */
>> +#define EXT4_DEF_PROJID 0
>> +
>> #define EXT4_DEF_INODE_READAHEAD_BLKS 32
>>
>> /*
>> @@ -2133,6 +2147,9 @@ extern int ext4_zero_partial_blocks(hand
>> loff_t lstart, loff_t lend);
>> extern int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
>> extern qsize_t *ext4_get_reserved_space(struct inode *inode);
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
>> +#endif
>> extern void ext4_da_update_reserve_space(struct inode *inode,
>> int used, int quota_claim);
>>
>> Index: linux.git/fs/ext4/inode.c
>> ===================================================================
>> --- linux.git.orig/fs/ext4/inode.c
>> +++ linux.git/fs/ext4/inode.c
>> @@ -4026,6 +4026,47 @@ static inline void ext4_iget_extra_inode
>> EXT4_I(inode)->i_inline_off = 0;
>> }
>>
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +static int ext4_inode_projid_get(struct inode *inode, struct ext4_inode *raw,
>> + struct ext4_inode_info *ei, projid_t *projid)
>> +{
>> + BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> + EXT4_FEATURE_RO_COMPAT_PROJECT));
>> +
>> + if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
>> + EXT4_FITS_IN_INODE(raw, ei, i_projid)) {
>> + *projid = (projid_t)le32_to_cpu(raw->i_projid);
>> + return 0;
>> + }
>> + return -EFBIG;
>> +}
> Maybe you should just return the default project ID for inodes that
> cannot store one?
Sure, on the "get" side that would probably be OK.
>> +static int ext4_inode_projid_set(struct inode *inode, struct ext4_inode *raw,
>> + struct ext4_inode_info *ei, projid_t projid)
>> +{
>> + BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> + EXT4_FEATURE_RO_COMPAT_PROJECT));
>> +
>> + if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
>> + EXT4_FITS_IN_INODE(raw, ei, i_projid)) {
>> + raw->i_projid = cpu_to_le32(projid);
>> + return 0;
>> + }
>> + return -EFBIG;
>> +}
>
> The test whether project ID fits into an inode should be done when
> userspace tries to set project ID for the inode in the ioctl. Thus when
> we are trying to write the inode and projid doesn't fit into the inode, we
> know projid is the default one and we can just avoid storing it.
> ext4_inode_projid_get() will then properly interpret this as the default
> projid. This way we can make project ids work reasonably for filesystems
> where inode needn't have enough space.
We also have patches for e2fsprogs to allow e2fsck to resize the
i_extra_isize on existing inodes to ensure that there is always
enough space in the inode to store the project quota.
Cheers, Andreas
>> +int ext4_get_projid(struct inode *inode, kprojid_t *projid)
>> +{
>> + if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> + EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>> + return -ENOTSUPP;
>> + }
>> +
>> + *projid = EXT4_I(inode)->i_projid;
>> + return 0;
>> +}
>> +#endif
>> +
> ...
>
> Honza
> --
> Jan Kara <jack@...e.cz>
> SUSE Labs, CR
Cheers, Andreas
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists