linux-ext4 - Re: [PATCH] jbd2: store more accurate errno in superblock when possible

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: <9c0b71f1-0cb3-4777-a9d7-dc9510fbb760@huaweicloud.com>
Date: Sat, 25 Oct 2025 10:04:05 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Wengang Wang <wen.gang.wang@...cle.com>, linux-ext4@...r.kernel.org
Cc: tytso@....edu, adilger.kernel@...ger.ca
Subject: Re: [PATCH] jbd2: store more accurate errno in superblock when
 possible

On 10/25/2025 3:35 AM, Wengang Wang wrote:
> When jbd2_journal_abort() is called, the provided error code is stored
> in the journal superblock. Some existing calls hard-code -EIO even when
> the actual failure is not I/O related.
> 
> This patch updates those calls to pass more accurate error codes,
> allowing the superblock to record the true cause of failure. This helps
> improve diagnostics and debugging clarity when analyzing journal aborts.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@...cle.com>

Thank you for the patch, it makes sense to me. I have just one minor
comment below.

> ---
>  fs/ext4/super.c       |  4 ++--
>  fs/jbd2/checkpoint.c  |  2 +-
>  fs/jbd2/journal.c     | 15 +++++++++------
>  fs/jbd2/transaction.c |  5 +++--
>  4 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 33e7c08c9529..baf1098cac63 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -698,7 +698,7 @@ static void ext4_handle_error(struct super_block *sb, bool force_ro, int error,
>  		WARN_ON_ONCE(1);
>  
>  	if (!continue_fs && !ext4_emergency_ro(sb) && journal)
> -		jbd2_journal_abort(journal, -EIO);
> +		jbd2_journal_abort(journal, error);
		                            ^^^^^
		                            this should be -error

The others look good to me.

Thanks,
Yi.

>  
>  	if (!bdev_read_only(sb->s_bdev)) {
>  		save_error_info(sb, error, ino, block, func, line);
> @@ -5842,7 +5842,7 @@ static int ext4_journal_bmap(journal_t *journal, sector_t *block)
>  		ext4_msg(journal->j_inode->i_sb, KERN_CRIT,
>  			 "journal bmap failed: block %llu ret %d\n",
>  			 *block, ret);
> -		jbd2_journal_abort(journal, ret ? ret : -EIO);
> +		jbd2_journal_abort(journal, ret ? ret : -EFSCORRUPTED);
>  		return ret;
>  	}
>  	*block = map.m_pblk;
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 2d0719bf6d87..de89c5bef607 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -113,7 +113,7 @@ __releases(&journal->j_state_lock)
>  				       "journal space in %s\n", __func__,
>  				       journal->j_devname);
>  				WARN_ON(1);
> -				jbd2_journal_abort(journal, -EIO);
> +				jbd2_journal_abort(journal, -ENOSPC);
>  			}
>  			write_lock(&journal->j_state_lock);
>  		} else {
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..d965dc0b9a59 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -937,8 +937,8 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
>  			printk(KERN_ALERT "%s: journal block not found "
>  					"at offset %lu on %s\n",
>  			       __func__, blocknr, journal->j_devname);
> +			jbd2_journal_abort(journal, ret ? ret : -EFSCORRUPTED);
>  			err = -EIO;
> -			jbd2_journal_abort(journal, err);
>  		} else {
>  			*retp = block;
>  		}
> @@ -1858,8 +1858,9 @@ int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
>  
>  	if (is_journal_aborted(journal))
>  		return -EIO;
> -	if (jbd2_check_fs_dev_write_error(journal)) {
> -		jbd2_journal_abort(journal, -EIO);
> +	ret = jbd2_check_fs_dev_write_error(journal);
> +	if (ret) {
> +		jbd2_journal_abort(journal, ret);
>  		return -EIO;
>  	}
>  
> @@ -2156,9 +2157,11 @@ int jbd2_journal_destroy(journal_t *journal)
>  	 * failed to write back to the original location, otherwise the
>  	 * filesystem may become inconsistent.
>  	 */
> -	if (!is_journal_aborted(journal) &&
> -	    jbd2_check_fs_dev_write_error(journal))
> -		jbd2_journal_abort(journal, -EIO);
> +	if (!is_journal_aborted(journal)) {
> +		int ret = jbd2_check_fs_dev_write_error(journal);
> +		if (ret)
> +			jbd2_journal_abort(journal, ret);
> +	}
>  
>  	if (journal->j_sb_buffer) {
>  		if (!is_journal_aborted(journal)) {
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 3e510564de6e..44dfaa9e7839 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1219,7 +1219,8 @@ int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
>  		return -EROFS;
>  
>  	journal = handle->h_transaction->t_journal;
> -	if (jbd2_check_fs_dev_write_error(journal)) {
> +	rc = jbd2_check_fs_dev_write_error(journal);
> +	if (rc) {
>  		/*
>  		 * If the fs dev has writeback errors, it may have failed
>  		 * to async write out metadata buffers in the background.
> @@ -1227,7 +1228,7 @@ int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
>  		 * it out again, which may lead to on-disk filesystem
>  		 * inconsistency. Aborting journal can avoid it happen.
>  		 */
> -		jbd2_journal_abort(journal, -EIO);
> +		jbd2_journal_abort(journal, rc);
>  		return -EIO;
>  	}
>