linux-ext4 - Re: [PATCH -next v3] jbd2: Fix data missing when reusing bh which is ready to be checkpointed

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]

Date:   Tue, 10 Jan 2023 09:17:39 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     Jan Kara <jack@...e.cz>
CC:     <tytso@....edu>, <jack@...e.com>, <linux-ext4@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <yi.zhang@...wei.com>,
        <libaokun1@...wei.com>, <zhanchengbin1@...wei.com>
Subject: Re: [PATCH -next v3] jbd2: Fix data missing when reusing bh which is
 ready to be checkpointed

> On Mon 09-01-23 21:45:45, Zhihao Cheng wrote >> Following process will make data lost and could lead to a 
filesystem>> corrupted problem:>>
[...]

> Just a suggestion for rephrasing of the comment below
> 
>> -		/*
>> -		 * In any case we need to clean the dirty flag and we must
>> -		 * do it under the buffer lock to be sure we don't race
>> -		 * with running write-out.
>> +		 * We need to clean the dirty flag and we must do it under the
>> +		 * buffer lock to be sure we don't race with running write-out.
>>   		 */
>>   		JBUFFER_TRACE(jh, "Journalling dirty buffer");
>>   		clear_buffer_dirty(bh);
>> +		/*
>> +		 * Setting jbddirty after clearing buffer dirty is necessary.
>> +		 * Function jbd2_journal_restart() could keep buffer on
>> +		 * BJ_Reserved list until the transaction committing, then the
>> +		 * buffer won't be dirtied by jbd2_journal_refile_buffer()
>> +		 * after committing, the buffer couldn't fall on disk even
>> +		 * last checkpoint finished, which may corrupt filesystem.
>> +		 */
> 
> As far as I understand you want to say:
> 		/*
> 		 * The buffer is going to be added to BJ_Reserved list now
> 		 * and nothing guarantees jbd2_journal_dirty_metadata()
> 		 * will be ever called for it. So we need to set jbddirty
> 		 * bit here to make sure the buffer is dirtied and written
> 		 * out when the journaling machinery is done with it.
> 		 */
> 
>>   		set_buffer_jbddirty(bh);
>>   	}
> 

Yes. The comment looks better than v3.