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]
Date:   Thu, 16 Feb 2023 17:17:25 +0800
From:   Baokun Li <libaokun1@...wei.com>
To:     "yebin (H)" <yebin10@...wei.com>, Ye Bin <yebin@...weicloud.com>,
        <tytso@....edu>, <adilger.kernel@...ger.ca>,
        <linux-ext4@...r.kernel.org>
CC:     <linux-kernel@...r.kernel.org>, <jack@...e.cz>
Subject: Re: [PATCH v3 1/2] ext4: commit super block if fs record error when
 journal record without error

On 2023/2/16 15:44, yebin (H) wrote:
>
>
> On 2023/2/16 15:17, Baokun Li wrote:
>> On 2023/2/14 10:29, Ye Bin wrote:
>>> From: Ye Bin <yebin10@...wei.com>
>>>
>>> Now, 'es->s_state' maybe covered by recover journal. And journal errno
>>> maybe not recorded in journal sb as IO error. ext4_update_super() only
>>> update error information when 'sbi->s_add_error_count' large than zero.
>>> Then 'EXT4_ERROR_FS' flag maybe lost.
>>> To solve above issue commit error information after recover journal.
>>>
>>> Signed-off-by: Ye Bin <yebin10@...wei.com>
>>> ---
>>>   fs/ext4/super.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>> index dc3907dff13a..b94754ba8556 100644
>>> --- a/fs/ext4/super.c
>>> +++ b/fs/ext4/super.c
>>> @@ -5932,6 +5932,18 @@ static int ext4_load_journal(struct 
>>> super_block *sb,
>>>           goto err_out;
>>>       }
>>>   +    if (unlikely(es->s_error_count && 
>>> !jbd2_journal_errno(journal) &&
>>> +             !(le16_to_cpu(es->s_state) & EXT4_ERROR_FS))) {
>>> +        EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
>>> +        es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
>>> +        err = ext4_commit_super(sb);
>>> +        if (err) {
>>> +            ext4_msg(sb, KERN_ERR,
>>> +                 "Failed to commit error information, please repair 
>>> fs force!");
>>> +            goto err_out;
>>> +        }
>>> +    }
>>> +
>>>       EXT4_SB(sb)->s_journal = journal;
>>>       err = ext4_clear_journal_err(sb, es);
>>>       if (err) {
>> I think we don't need such a complicated judgment, after the journal 
>> replay and saving the error info,
>> if there is EXT4_ERROR_FS flag in ext4_sb_info->s_mount_state, just 
>> add this flag directly to es->s_state.
>> This way the EXT4_ERROR_FS flag and the error message will be written 
>> to disk the next time
>
> Thanks for your suggestion. There are two reasons for this:
> 1. We want to write the error mark to the disk as soon as possible.
> 2. Here we deal with the case where there is no error mark bit but 
> there is an error record.
> In this case, the file system should be marked with an error and the 
> user should be prompted.
The EXT4_ERROR_FS flag is always written to disk at the same time as the 
error info,
except when the journal is replayed. During journal replay the error 
info is additionally
copied to disk first, and the EXT4_ERROR_FS flag in the sbi is not 
written to disk until
the ext4_put_super() is called. It is only when a failure occurs during 
this time that
there is an error info but no EXT4_ERROR_FS flag. So we just need to 
make sure that
the EXT4_ERROR_FS flag is also written to disk at the same time as the 
error info
after the journal replay.
>> ext4_commit_super() is executed. The code change is as follows:
>>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 260c1b3e3ef2..341b11c589b3 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -5935,6 +5935,7 @@ static int ext4_load_journal(struct super_block 
>> *sb,
>>                         memcpy(((char *) es) + EXT4_S_ERR_START,
>>                                save, EXT4_S_ERR_LEN);
>>                 kfree(save);
>> +               es->s_state |= cpu_to_le16(EXT4_SB(sb)->s_mount_state 
>> & EXT4_ERROR_FS);
>>         }
>>
>>         if (err) {
>>
>
>
>
-- 
With Best Regards,
Baokun Li
.

Powered by blists - more mailing lists