linux-ext4 - Re: [PATCH 1/6] jbd2: Issue cache flush after checkpointing even with internal journal

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: <20120209030551.GH18461@thunk.org>
Date:	Wed, 8 Feb 2012 22:05:51 -0500
From:	Ted Ts'o <tytso@....edu>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/6] jbd2: Issue cache flush after checkpointing even
 with internal journal

Hi Jan,

Am I missing something?  In the original code, we figure out the block
# of the tail of the journal while holding the j_state_lock for
writing, and we hold the lock until journal->j_tail is updated.

In your proposed replacement code, you call
jbd2_journal_get_log_tail() to determine the block #, but you aren't
holding any locks.  jbd2_journal_get_log_tail() grabs a read lock to
figure out the block number, but then drops the lock before it
returns.  So then journal->j_tail gets updated by
jbd2_journal_update_tail() --- using the block # determined by
jbd2_journal_get_log_tail(), but we've released the lock, so can we
guarantee the block number is still accurate?

In particular, since jbd2_cleanup_journal_tail() is now not holding
any locks, what if it is racing against itself?  I can't quite see
race that would lead to something horrible happening, but my spidey
sense is tingling....

Also:

> +/*
> + * Update information in journal about log tail. The function returns 1 if
> + * tail was updated, 0 otherwise. If 1 is returned, caller *must* write
> + * journal superblock before next transaction commit is started.
> + */

If jbd2_update_log_tail() returns 1, how is this enforced?  The caller
can issue a journal superblocok update, sure, but there's no locking
to prevent some other process from immediately starting a new
transaction?

Again, am I missing something?

Regards,

							- Ted
--
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