diff mbox series

[v3,2/4] ext4: add helper to check quota inums

Message ID 20221026042310.3839669-3-libaokun1@huawei.com
State Awaiting Upstream
Headers show
Series ext4: fix two bug_on in __es_tree_search | expand

Commit Message

Baokun Li Oct. 26, 2022, 4:23 a.m. UTC
Before quota is enabled, a check on the preset quota inums in
ext4_super_block is added to prevent wrong quota inodes from being loaded.
In addition, when the quota fails to be enabled, the quota type and quota
inum are printed to facilitate fault locating.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/super.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Comments

Jan Kara Oct. 26, 2022, 9:38 a.m. UTC | #1
On Wed 26-10-22 12:23:08, Baokun Li wrote:
> Before quota is enabled, a check on the preset quota inums in
> ext4_super_block is added to prevent wrong quota inodes from being loaded.
> In addition, when the quota fails to be enabled, the quota type and quota
> inum are printed to facilitate fault locating.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Makes sense. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/super.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 34b78f380968..0b4060d52d63 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6885,6 +6885,20 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
>  	return err;
>  }
>  
> +static inline bool ext4_check_quota_inum(int type, unsigned long qf_inum)
> +{
> +	switch (type) {
> +	case USRQUOTA:
> +		return qf_inum == EXT4_USR_QUOTA_INO;
> +	case GRPQUOTA:
> +		return qf_inum == EXT4_GRP_QUOTA_INO;
> +	case PRJQUOTA:
> +		return qf_inum >= EXT4_GOOD_OLD_FIRST_INO;
> +	default:
> +		BUG();
> +	}
> +}
> +
>  static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
>  			     unsigned int flags)
>  {
> @@ -6901,9 +6915,16 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
>  	if (!qf_inums[type])
>  		return -EPERM;
>  
> +	if (!ext4_check_quota_inum(type, qf_inums[type])) {
> +		ext4_error(sb, "Bad quota inum: %lu, type: %d",
> +				qf_inums[type], type);
> +		return -EUCLEAN;
> +	}
> +
>  	qf_inode = ext4_iget(sb, qf_inums[type], EXT4_IGET_SPECIAL);
>  	if (IS_ERR(qf_inode)) {
> -		ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
> +		ext4_error(sb, "Bad quota inode: %lu, type: %d",
> +				qf_inums[type], type);
>  		return PTR_ERR(qf_inode);
>  	}
>  
> @@ -6942,8 +6963,9 @@ int ext4_enable_quotas(struct super_block *sb)
>  			if (err) {
>  				ext4_warning(sb,
>  					"Failed to enable quota tracking "
> -					"(type=%d, err=%d). Please run "
> -					"e2fsck to fix.", type, err);
> +					"(type=%d, err=%d, ino=%lu). "
> +					"Please run e2fsck to fix.", type,
> +					err, qf_inums[type]);
>  				for (type--; type >= 0; type--) {
>  					struct inode *inode;
>  
> -- 
> 2.31.1
>
Jason Yan Oct. 26, 2022, 1:57 p.m. UTC | #2
On 2022/10/26 12:23, Baokun Li wrote:
> Before quota is enabled, a check on the preset quota inums in
> ext4_super_block is added to prevent wrong quota inodes from being loaded.
> In addition, when the quota fails to be enabled, the quota type and quota
> inum are printed to facilitate fault locating.
> 
> Signed-off-by: Baokun Li<libaokun1@huawei.com>
> ---
>   fs/ext4/super.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)

Looks good,
Reviewed-by: Jason Yan <yanaijie@huawei.com>
diff mbox series

Patch

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 34b78f380968..0b4060d52d63 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6885,6 +6885,20 @@  static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 	return err;
 }
 
+static inline bool ext4_check_quota_inum(int type, unsigned long qf_inum)
+{
+	switch (type) {
+	case USRQUOTA:
+		return qf_inum == EXT4_USR_QUOTA_INO;
+	case GRPQUOTA:
+		return qf_inum == EXT4_GRP_QUOTA_INO;
+	case PRJQUOTA:
+		return qf_inum >= EXT4_GOOD_OLD_FIRST_INO;
+	default:
+		BUG();
+	}
+}
+
 static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
 			     unsigned int flags)
 {
@@ -6901,9 +6915,16 @@  static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
 	if (!qf_inums[type])
 		return -EPERM;
 
+	if (!ext4_check_quota_inum(type, qf_inums[type])) {
+		ext4_error(sb, "Bad quota inum: %lu, type: %d",
+				qf_inums[type], type);
+		return -EUCLEAN;
+	}
+
 	qf_inode = ext4_iget(sb, qf_inums[type], EXT4_IGET_SPECIAL);
 	if (IS_ERR(qf_inode)) {
-		ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
+		ext4_error(sb, "Bad quota inode: %lu, type: %d",
+				qf_inums[type], type);
 		return PTR_ERR(qf_inode);
 	}
 
@@ -6942,8 +6963,9 @@  int ext4_enable_quotas(struct super_block *sb)
 			if (err) {
 				ext4_warning(sb,
 					"Failed to enable quota tracking "
-					"(type=%d, err=%d). Please run "
-					"e2fsck to fix.", type, err);
+					"(type=%d, err=%d, ino=%lu). "
+					"Please run e2fsck to fix.", type,
+					err, qf_inums[type]);
 				for (type--; type >= 0; type--) {
 					struct inode *inode;