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] [day] [month] [year] [list]
Message-ID: <F3159D0F-9DB8-4121-A9E0-51764C352BC3@oracle.com>
Date: Sat, 25 Oct 2025 03:33:28 +0000
From: Wengang Wang <wen.gang.wang@...cle.com>
To: Zhang Yi <yi.zhang@...weicloud.com>
CC: "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        "tytso@....edu"
	<tytso@....edu>,
        "adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>
Subject: Re: [PATCH] jbd2: store more accurate errno in superblock when
 possible

Hi Yi,
Thanks for review! 

> On Oct 24, 2025, at 7:04 PM, Zhang Yi <yi.zhang@...weicloud.com> wrote:
> 
> 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

Yes, nice catch!

Thanks,
Wengang

> 
> 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;
>> }