Skip to content

Commit 9c52057

Browse files
author
Chris Mason
committed
Btrfs: fix hash overflow handling
The handling for directory crc hash overflows was fairly obscure, split_leaf returns EOVERFLOW when we try to extend the item and that is supposed to bubble up to userland. For a while it did so, but along the way we added better handling of errors and forced the FS readonly if we hit IO errors during the directory insertion. Along the way, we started testing only for EEXIST and the EOVERFLOW case was dropped. The end result is that we may force the FS readonly if we catch a directory hash bucket overflow. This fixes a few problem spots. First I add tests for EOVERFLOW in the places where we can safely just return the error up the chain. btrfs_rename is harder though, because it tries to insert the new directory item only after it has already unlinked anything the rename was going to overwrite. Rather than adding very complex logic, I added a helper to test for the hash overflow case early while it is still safe to bail out. Snapshot and subvolume creation had a similar problem, so they are using the new helper now too. Signed-off-by: Chris Mason <chris.mason@fusionio.com> Reported-by: Pascal Junod <pascal@junod.info>
1 parent c64c2bd commit 9c52057

File tree

5 files changed

+95
-2
lines changed

5 files changed

+95
-2
lines changed

Diff for: fs/btrfs/ctree.h

+2
Original file line numberDiff line numberDiff line change
@@ -3283,6 +3283,8 @@ void btrfs_update_root_times(struct btrfs_trans_handle *trans,
32833283
struct btrfs_root *root);
32843284

32853285
/* dir-item.c */
3286+
int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
3287+
const char *name, int name_len);
32863288
int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
32873289
struct btrfs_root *root, const char *name,
32883290
int name_len, struct inode *dir,

Diff for: fs/btrfs/dir-item.c

+59
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,65 @@ struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
213213
return btrfs_match_dir_item_name(root, path, name, name_len);
214214
}
215215

216+
int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
217+
const char *name, int name_len)
218+
{
219+
int ret;
220+
struct btrfs_key key;
221+
struct btrfs_dir_item *di;
222+
int data_size;
223+
struct extent_buffer *leaf;
224+
int slot;
225+
struct btrfs_path *path;
226+
227+
228+
path = btrfs_alloc_path();
229+
if (!path)
230+
return -ENOMEM;
231+
232+
key.objectid = dir;
233+
btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY);
234+
key.offset = btrfs_name_hash(name, name_len);
235+
236+
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
237+
238+
/* return back any errors */
239+
if (ret < 0)
240+
goto out;
241+
242+
/* nothing found, we're safe */
243+
if (ret > 0) {
244+
ret = 0;
245+
goto out;
246+
}
247+
248+
/* we found an item, look for our name in the item */
249+
di = btrfs_match_dir_item_name(root, path, name, name_len);
250+
if (di) {
251+
/* our exact name was found */
252+
ret = -EEXIST;
253+
goto out;
254+
}
255+
256+
/*
257+
* see if there is room in the item to insert this
258+
* name
259+
*/
260+
data_size = sizeof(*di) + name_len + sizeof(struct btrfs_item);
261+
leaf = path->nodes[0];
262+
slot = path->slots[0];
263+
if (data_size + btrfs_item_size_nr(leaf, slot) +
264+
sizeof(struct btrfs_item) > BTRFS_LEAF_DATA_SIZE(root)) {
265+
ret = -EOVERFLOW;
266+
} else {
267+
/* plenty of insertion room */
268+
ret = 0;
269+
}
270+
out:
271+
btrfs_free_path(path);
272+
return ret;
273+
}
274+
216275
/*
217276
* lookup a directory item based on index. 'dir' is the objectid
218277
* we're searching in, and 'mod' tells us if you plan on deleting the

Diff for: fs/btrfs/inode.c

+23-1
Original file line numberDiff line numberDiff line change
@@ -4885,7 +4885,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
48854885
ret = btrfs_insert_dir_item(trans, root, name, name_len,
48864886
parent_inode, &key,
48874887
btrfs_inode_type(inode), index);
4888-
if (ret == -EEXIST)
4888+
if (ret == -EEXIST || ret == -EOVERFLOW)
48894889
goto fail_dir_item;
48904890
else if (ret) {
48914891
btrfs_abort_transaction(trans, root, ret);
@@ -7336,6 +7336,28 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
73367336
if (S_ISDIR(old_inode->i_mode) && new_inode &&
73377337
new_inode->i_size > BTRFS_EMPTY_DIR_SIZE)
73387338
return -ENOTEMPTY;
7339+
7340+
7341+
/* check for collisions, even if the name isn't there */
7342+
ret = btrfs_check_dir_item_collision(root, new_dir->i_ino,
7343+
new_dentry->d_name.name,
7344+
new_dentry->d_name.len);
7345+
7346+
if (ret) {
7347+
if (ret == -EEXIST) {
7348+
/* we shouldn't get
7349+
* eexist without a new_inode */
7350+
if (!new_inode) {
7351+
WARN_ON(1);
7352+
return ret;
7353+
}
7354+
} else {
7355+
/* maybe -EOVERFLOW */
7356+
return ret;
7357+
}
7358+
}
7359+
ret = 0;
7360+
73397361
/*
73407362
* we're using rename to replace one file with another.
73417363
* and the replacement file is large. Start IO on it now so

Diff for: fs/btrfs/ioctl.c

+10
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,16 @@ static noinline int btrfs_mksubvol(struct path *parent,
710710
if (error)
711711
goto out_dput;
712712

713+
/*
714+
* even if this name doesn't exist, we may get hash collisions.
715+
* check for them now when we can safely fail
716+
*/
717+
error = btrfs_check_dir_item_collision(BTRFS_I(dir)->root,
718+
dir->i_ino, name,
719+
namelen);
720+
if (error)
721+
goto out_dput;
722+
713723
down_read(&BTRFS_I(dir)->root->fs_info->subvol_sem);
714724

715725
if (btrfs_root_refs(&BTRFS_I(dir)->root->root_item) == 0)

Diff for: fs/btrfs/transaction.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
11901190
parent_inode, &key,
11911191
BTRFS_FT_DIR, index);
11921192
/* We have check then name at the beginning, so it is impossible. */
1193-
BUG_ON(ret == -EEXIST);
1193+
BUG_ON(ret == -EEXIST || ret == -EOVERFLOW);
11941194
if (ret) {
11951195
btrfs_abort_transaction(trans, root, ret);
11961196
goto fail;

0 commit comments

Comments
 (0)