linux-ext4 - Re: [PATCH v3 2/4] ext4, jbd2: ensure panic when aborting with zero errno

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: <20191204125213.GG8206@quack2.suse.cz>
Date:   Wed, 4 Dec 2019 13:52:13 +0100
From:   Jan Kara <jack@...e.cz>
To:     "zhangyi (F)" <yi.zhang@...wei.com>
Cc:     linux-ext4@...r.kernel.org, jack@...e.com, tytso@....edu,
        adilger.kernel@...ger.ca, liangyun2@...wei.com,
        luoshijie1@...wei.com
Subject: Re: [PATCH v3 2/4] ext4, jbd2: ensure panic when aborting with zero
 errno

On Wed 04-12-19 20:46:12, zhangyi (F) wrote:
> JBD2_REC_ERR flag used to indicate the errno has been updated when jbd2
> aborted, and then __ext4_abort() and ext4_handle_error() can invoke
> panic if ERRORS_PANIC is specified. But if the journal has been aborted
> with zero errno, jbd2_journal_abort() didn't set this flag so we can
> no longer panic. Fix this by always record the proper errno in the
> journal superblock.
> 
> Fixes: 4327ba52afd03 ("ext4, jbd2: ensure entering into panic after recording an error in superblock")
> Signed-off-by: zhangyi (F) <yi.zhang@...wei.com>

Looks good to me. You can add:

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

								Honza

> ---
>  fs/jbd2/checkpoint.c |  2 +-
>  fs/jbd2/journal.c    | 15 ++++-----------
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index a1909066bde6..62cf497f18eb 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -164,7 +164,7 @@ void __jbd2_log_wait_for_space(journal_t *journal)
>  				       "journal space in %s\n", __func__,
>  				       journal->j_devname);
>  				WARN_ON(1);
> -				jbd2_journal_abort(journal, 0);
> +				jbd2_journal_abort(journal, -EIO);
>  			}
>  			write_lock(&journal->j_state_lock);
>  		} else {
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 1c58859aa592..b2d6e7666d0f 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2118,12 +2118,10 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>  
>  	__jbd2_journal_abort_hard(journal);
>  
> -	if (errno) {
> -		jbd2_journal_update_sb_errno(journal);
> -		write_lock(&journal->j_state_lock);
> -		journal->j_flags |= JBD2_REC_ERR;
> -		write_unlock(&journal->j_state_lock);
> -	}
> +	jbd2_journal_update_sb_errno(journal);
> +	write_lock(&journal->j_state_lock);
> +	journal->j_flags |= JBD2_REC_ERR;
> +	write_unlock(&journal->j_state_lock);
>  }
>  
>  /**
> @@ -2165,11 +2163,6 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>   * failure to disk.  ext3_error, for example, now uses this
>   * functionality.
>   *
> - * Errors which originate from within the journaling layer will NOT
> - * supply an errno; a null errno implies that absolutely no further
> - * writes are done to the journal (unless there are any already in
> - * progress).
> - *
>   */
>  
>  void jbd2_journal_abort(journal_t *journal, int errno)
> -- 
> 2.17.2
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR