Skip to content

Commit 98cf21c

Browse files
saprojtorvalds
authored andcommitted
hfsplus: fix B-tree corruption after insertion at position 0
Fix B-tree corruption when a new record is inserted at position 0 in the node in hfs_brec_insert(). In this case a hfs_brec_update_parent() is called to update the parent index node (if exists) and it is passed hfs_find_data with a search_key containing a newly inserted key instead of the key to be updated. This results in an inconsistent index node. The bug reproduces on my machine after an extents overflow record for the catalog file (CNID=4) is inserted into the extents overflow B-tree. Because of a low (reserved) value of CNID=4, it has to become the first record in the first leaf node. The resulting first leaf node is correct: ---------------------------------------------------- | key0.CNID=4 | key1.CNID=123 | key2.CNID=456, ... | ---------------------------------------------------- But the parent index key0 still contains the previous key CNID=123: ----------------------- | key0.CNID=123 | ... | ----------------------- A change in hfs_brec_insert() makes hfs_brec_update_parent() work correctly by preventing it from getting fd->record=-1 value from __hfs_brec_find(). Along the way, I removed duplicate code with unification of the if condition. The resulting code is equivalent to the original code because node is never 0. Also hfs_brec_update_parent() will now return an error after getting a negative fd->record value. However, the return value of hfs_brec_update_parent() is not checked anywhere in the file and I'm leaving it unchanged by this patch. brec.c lacks error checking after some other calls too, but this issue is of less importance than the one being fixed by this patch. Signed-off-by: Sergei Antonov <saproj@gmail.com> Cc: Joe Perches <joe@perches.com> Reviewed-by: Vyacheslav Dubeyko <slava@dubeyko.com> Acked-by: Hin-Tak Leung <htl10@users.sourceforge.net> Cc: Anton Altaparmakov <aia21@cam.ac.uk> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christoph Hellwig <hch@infradead.org> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 1f31e1b commit 98cf21c

File tree

1 file changed

+11
-9
lines changed

1 file changed

+11
-9
lines changed

fs/hfsplus/brec.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,16 @@ int hfs_brec_insert(struct hfs_find_data *fd, void *entry, int entry_len)
131131
hfs_bnode_write(node, entry, data_off + key_len, entry_len);
132132
hfs_bnode_dump(node);
133133

134-
if (new_node) {
135-
/* update parent key if we inserted a key
136-
* at the start of the first node
137-
*/
138-
if (!rec && new_node != node)
139-
hfs_brec_update_parent(fd);
134+
/*
135+
* update parent key if we inserted a key
136+
* at the start of the node and it is not the new node
137+
*/
138+
if (!rec && new_node != node) {
139+
hfs_bnode_read_key(node, fd->search_key, data_off + size);
140+
hfs_brec_update_parent(fd);
141+
}
140142

143+
if (new_node) {
141144
hfs_bnode_put(fd->bnode);
142145
if (!new_node->parent) {
143146
hfs_btree_inc_height(tree);
@@ -168,9 +171,6 @@ int hfs_brec_insert(struct hfs_find_data *fd, void *entry, int entry_len)
168171
goto again;
169172
}
170173

171-
if (!rec)
172-
hfs_brec_update_parent(fd);
173-
174174
return 0;
175175
}
176176

@@ -370,6 +370,8 @@ static int hfs_brec_update_parent(struct hfs_find_data *fd)
370370
if (IS_ERR(parent))
371371
return PTR_ERR(parent);
372372
__hfs_brec_find(parent, fd, hfs_find_rec_by_key);
373+
if (fd->record < 0)
374+
return -ENOENT;
373375
hfs_bnode_dump(parent);
374376
rec = fd->record;
375377

0 commit comments

Comments
 (0)