linux-kernel - Re: [PATCH v2 2/3] ext4: record error when detect abnormal 'i_reserved_data

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3xDY4GVAF+eKAqY@debian-BULLSEYE-live-builder-AMD64>
Date:   Mon, 21 Nov 2022 22:34:59 -0500
From:   Eric Whitney <enwlinux@...il.com>
To:     Ye Bin <yebin@...weicloud.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        jack@...e.cz, Ye Bin <yebin10@...wei.com>
Subject: Re: [PATCH v2 2/3] ext4: record error when detect abnormal
 'i_reserved_data_blocks'

* Ye Bin <yebin@...weicloud.com>:
> From: Ye Bin <yebin10@...wei.com>
> 
> If 'i_reserved_data_blocks' is not cleared which mean something wrong with
> code, free space accounting is likely wrong, according to Jan Kara's advice
> use ext4_error() to record this abnormal let fsck to repair and also we can
> capture this issue.
> 
> Signed-off-by: Ye Bin <yebin10@...wei.com>
> ---
>  fs/ext4/super.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 0690e2e0b74d..3d30007502a4 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1387,10 +1387,9 @@ static void ext4_destroy_inode(struct inode *inode)
>  	}
>  
>  	if (EXT4_I(inode)->i_reserved_data_blocks)
> -		ext4_msg(inode->i_sb, KERN_ERR,

Per the coding standard, IIRC, the string should not be split across lines
for "greppability", so it should remain as is.  It's always good to run
checkpatch to catch stuff like this.

> -			 "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
> -			 inode->i_ino, EXT4_I(inode),
> -			 EXT4_I(inode)->i_reserved_data_blocks);
> +		ext4_error(inode->i_sb, "Inode %lu (%p) i_reserved_data_blocks"
> +			   " (%u) not cleared!", inode->i_ino, EXT4_I(inode),
> +			   EXT4_I(inode)->i_reserved_data_blocks);
>  }
>  
>  static void init_once(void *foo)

This is an improvement over the first version.  If i_reserved_data_blocks is
non-zero, something is definitely broken, but it's perhaps less likely to
indicate file system damage than if it hits zero while there are still
outstanding delayed blocks (handled well elsewhere).  So, it's not clear
we need to escalate handling this case but it doesn't hurt, either.

Eric

> -- 
> 2.31.1
>