linux-ext4 - Re: [PATCH 3/4] jbd: abort when failed to log metadata buffers (rebased)

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: <482D6171.1090209@hitachi.com>
Date:	Fri, 16 May 2008 19:26:57 +0900
From:	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
To:	Jan Kara <jack@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, sct@...hat.com,
	adilger@...sterfs.com, linux-kernel@...r.kernel.org,
	linux-ext4@...r.kernel.org, Josef Bacik <jbacik@...hat.com>,
	Mingming Cao <cmm@...ibm.com>,
	Satoshi OSHIMA <satoshi.oshima.fk@...achi.com>,
	sugita <yumiko.sugita.yf@...achi.com>
Subject: Re: [PATCH 3/4] jbd: abort when failed to log metadata
    buffers	(rebased)

Hi,

Thank you for review.

Jan Kara wrote:

> On Wed 14-05-08 13:49:51, Hidehiro Kawai wrote:
> 
>>Subject: [PATCH 3/4] jbd: abort when failed to log metadata buffers
>>
>>If we failed to write metadata buffers to the journal space and
>>succeeded to write the commit record, stale data can be written
>>back to the filesystem as metadata in the recovery phase.
>>
>>To avoid this, when we failed to write out metadata buffers,
>>abort the journal before writing the commit record.
>>
>>Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
>>---
>> fs/jbd/commit.c |    3 +++
>> 1 file changed, 3 insertions(+)
>>
>>Index: linux-2.6.26-rc2/fs/jbd/commit.c
>>===================================================================
>>--- linux-2.6.26-rc2.orig/fs/jbd/commit.c
>>+++ linux-2.6.26-rc2/fs/jbd/commit.c
>>@@ -703,6 +703,9 @@ wait_for_iobuf:
>> 		__brelse(bh);
>> 	}
>> 
>>+	if (err)
>>+		journal_abort(journal, err);
>>+
>> 	J_ASSERT (commit_transaction->t_shadow_list == NULL);
> 
>   Shouldn't this rather be further just before
> journal_write_commit_record()? We should abort also if writing revoke
> records etc. failed, shouldn't we?

Unlike metadata blocks, each revoke block has a descriptor with the
sequence number of the commiting transaction.  If we failed to write
a revoke block, there should be an old control block, metadata block,
or zero-filled block where we tried to write the revoke block.
In the recovery process, this old invalid block is detected by
checking its magic number and sequence number, then the transaction
is ignored even if we have succeeded to write the commit record.
So I think we don't need to check for errors just after writing
revoke records.

Thanks,
 
>> 	jbd_debug(3, "JBD: commit phase 5\n");
>>

-- 
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html