]>
Commit | Line | Data |
---|---|---|
04fd09d4 SL |
1 | From 318b76fb931301f0dbc07ee98edf8342768246df Mon Sep 17 00:00:00 2001 |
2 | From: Theodore Ts'o <tytso@mit.edu> | |
3 | Date: Thu, 14 Feb 2019 16:27:14 -0500 | |
4 | Subject: jbd2: fix race when writing superblock | |
5 | ||
6 | [ Upstream commit 538bcaa6261b77e71d37f5596c33127c1a3ec3f7 ] | |
7 | ||
8 | The jbd2 superblock is lockless now, so there is probably a race | |
9 | condition between writing it so disk and modifing contents of it, which | |
10 | may lead to checksum error. The following race is the one case that we | |
11 | have captured. | |
12 | ||
13 | jbd2 fsstress | |
14 | jbd2_journal_commit_transaction | |
15 | jbd2_journal_update_sb_log_tail | |
16 | jbd2_write_superblock | |
17 | jbd2_superblock_csum_set jbd2_journal_revoke | |
18 | jbd2_journal_set_features(revork) | |
19 | modify superblock | |
20 | submit_bh(checksum incorrect) | |
21 | ||
22 | Fix this by locking the buffer head before modifing it. We always | |
23 | write the jbd2 superblock after we modify it, so this just means | |
24 | calling the lock_buffer() a little earlier. | |
25 | ||
26 | This checksum corruption problem can be reproduced by xfstests | |
27 | generic/475. | |
28 | ||
29 | Reported-by: zhangyi (F) <yi.zhang@huawei.com> | |
30 | Suggested-by: Jan Kara <jack@suse.cz> | |
31 | Signed-off-by: Theodore Ts'o <tytso@mit.edu> | |
32 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
33 | --- | |
34 | fs/jbd2/journal.c | 52 ++++++++++++++++++++++++----------------------- | |
35 | 1 file changed, 27 insertions(+), 25 deletions(-) | |
36 | ||
37 | diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c | |
38 | index 61d48f0c41a1..0c8f77db60e2 100644 | |
39 | --- a/fs/jbd2/journal.c | |
40 | +++ b/fs/jbd2/journal.c | |
41 | @@ -1343,6 +1343,10 @@ static int journal_reset(journal_t *journal) | |
42 | return jbd2_journal_start_thread(journal); | |
43 | } | |
44 | ||
45 | +/* | |
46 | + * This function expects that the caller will have locked the journal | |
47 | + * buffer head, and will return with it unlocked | |
48 | + */ | |
49 | static int jbd2_write_superblock(journal_t *journal, int write_flags) | |
50 | { | |
51 | struct buffer_head *bh = journal->j_sb_buffer; | |
52 | @@ -1352,7 +1356,6 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags) | |
53 | trace_jbd2_write_superblock(journal, write_flags); | |
54 | if (!(journal->j_flags & JBD2_BARRIER)) | |
55 | write_flags &= ~(REQ_FUA | REQ_PREFLUSH); | |
56 | - lock_buffer(bh); | |
57 | if (buffer_write_io_error(bh)) { | |
58 | /* | |
59 | * Oh, dear. A previous attempt to write the journal | |
60 | @@ -1411,6 +1414,7 @@ int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid, | |
61 | jbd_debug(1, "JBD2: updating superblock (start %lu, seq %u)\n", | |
62 | tail_block, tail_tid); | |
63 | ||
64 | + lock_buffer(journal->j_sb_buffer); | |
65 | sb->s_sequence = cpu_to_be32(tail_tid); | |
66 | sb->s_start = cpu_to_be32(tail_block); | |
67 | ||
68 | @@ -1441,18 +1445,17 @@ static void jbd2_mark_journal_empty(journal_t *journal, int write_op) | |
69 | journal_superblock_t *sb = journal->j_superblock; | |
70 | ||
71 | BUG_ON(!mutex_is_locked(&journal->j_checkpoint_mutex)); | |
72 | - read_lock(&journal->j_state_lock); | |
73 | - /* Is it already empty? */ | |
74 | - if (sb->s_start == 0) { | |
75 | - read_unlock(&journal->j_state_lock); | |
76 | + lock_buffer(journal->j_sb_buffer); | |
77 | + if (sb->s_start == 0) { /* Is it already empty? */ | |
78 | + unlock_buffer(journal->j_sb_buffer); | |
79 | return; | |
80 | } | |
81 | + | |
82 | jbd_debug(1, "JBD2: Marking journal as empty (seq %d)\n", | |
83 | journal->j_tail_sequence); | |
84 | ||
85 | sb->s_sequence = cpu_to_be32(journal->j_tail_sequence); | |
86 | sb->s_start = cpu_to_be32(0); | |
87 | - read_unlock(&journal->j_state_lock); | |
88 | ||
89 | jbd2_write_superblock(journal, write_op); | |
90 | ||
91 | @@ -1475,9 +1478,8 @@ void jbd2_journal_update_sb_errno(journal_t *journal) | |
92 | journal_superblock_t *sb = journal->j_superblock; | |
93 | int errcode; | |
94 | ||
95 | - read_lock(&journal->j_state_lock); | |
96 | + lock_buffer(journal->j_sb_buffer); | |
97 | errcode = journal->j_errno; | |
98 | - read_unlock(&journal->j_state_lock); | |
99 | if (errcode == -ESHUTDOWN) | |
100 | errcode = 0; | |
101 | jbd_debug(1, "JBD2: updating superblock error (errno %d)\n", errcode); | |
102 | @@ -1881,28 +1883,27 @@ int jbd2_journal_set_features (journal_t *journal, unsigned long compat, | |
103 | ||
104 | sb = journal->j_superblock; | |
105 | ||
106 | + /* Load the checksum driver if necessary */ | |
107 | + if ((journal->j_chksum_driver == NULL) && | |
108 | + INCOMPAT_FEATURE_ON(JBD2_FEATURE_INCOMPAT_CSUM_V3)) { | |
109 | + journal->j_chksum_driver = crypto_alloc_shash("crc32c", 0, 0); | |
110 | + if (IS_ERR(journal->j_chksum_driver)) { | |
111 | + printk(KERN_ERR "JBD2: Cannot load crc32c driver.\n"); | |
112 | + journal->j_chksum_driver = NULL; | |
113 | + return 0; | |
114 | + } | |
115 | + /* Precompute checksum seed for all metadata */ | |
116 | + journal->j_csum_seed = jbd2_chksum(journal, ~0, sb->s_uuid, | |
117 | + sizeof(sb->s_uuid)); | |
118 | + } | |
119 | + | |
120 | + lock_buffer(journal->j_sb_buffer); | |
121 | + | |
122 | /* If enabling v3 checksums, update superblock */ | |
123 | if (INCOMPAT_FEATURE_ON(JBD2_FEATURE_INCOMPAT_CSUM_V3)) { | |
124 | sb->s_checksum_type = JBD2_CRC32C_CHKSUM; | |
125 | sb->s_feature_compat &= | |
126 | ~cpu_to_be32(JBD2_FEATURE_COMPAT_CHECKSUM); | |
127 | - | |
128 | - /* Load the checksum driver */ | |
129 | - if (journal->j_chksum_driver == NULL) { | |
130 | - journal->j_chksum_driver = crypto_alloc_shash("crc32c", | |
131 | - 0, 0); | |
132 | - if (IS_ERR(journal->j_chksum_driver)) { | |
133 | - printk(KERN_ERR "JBD2: Cannot load crc32c " | |
134 | - "driver.\n"); | |
135 | - journal->j_chksum_driver = NULL; | |
136 | - return 0; | |
137 | - } | |
138 | - | |
139 | - /* Precompute checksum seed for all metadata */ | |
140 | - journal->j_csum_seed = jbd2_chksum(journal, ~0, | |
141 | - sb->s_uuid, | |
142 | - sizeof(sb->s_uuid)); | |
143 | - } | |
144 | } | |
145 | ||
146 | /* If enabling v1 checksums, downgrade superblock */ | |
147 | @@ -1914,6 +1915,7 @@ int jbd2_journal_set_features (journal_t *journal, unsigned long compat, | |
148 | sb->s_feature_compat |= cpu_to_be32(compat); | |
149 | sb->s_feature_ro_compat |= cpu_to_be32(ro); | |
150 | sb->s_feature_incompat |= cpu_to_be32(incompat); | |
151 | + unlock_buffer(journal->j_sb_buffer); | |
152 | ||
153 | return 1; | |
154 | #undef COMPAT_FEATURE_ON | |
155 | -- | |
156 | 2.19.1 | |
157 |