Skip to content

Commit 764433a

Browse files
Robbie Kofdmanana
authored andcommitted
Btrfs: send, fix invalid leaf accesses due to incorrect utimes operations
During an incremental send, if we have delayed rename operations for inodes that were children of directories which were removed in the send snapshot, we can end up accessing incorrect items in a leaf or accessing beyond the last item of the leaf due to issuing utimes operations for the removed inodes. Consider the following example: Parent snapshot: . (ino 256) |--- a/ (ino 257) | |--- c/ (ino 262) | |--- b/ (ino 258) | |--- d/ (ino 263) | |--- del/ (ino 261) |--- x/ (ino 259) |--- y/ (ino 260) Send snapshot: . (ino 256) |--- a/ (ino 257) | |--- b/ (ino 258) | |--- c/ (ino 262) | |--- y/ (ino 260) | |--- d/ (ino 263) |--- x/ (ino 259) 1) When processing inodes 259 and 260, we end up delaying their rename operations because their parents, inodes 263 and 262 respectively, were not yet processed and therefore not yet renamed; 2) When processing inode 262, its rename operation is issued and right after the rename operation for inode 260 is issued. However right after issuing the rename operation for inode 260, at send.c:apply_dir_move(), we issue utimes operations for all current and past parents of inode 260. This means we try to send a utimes operation for its old parent, inode 261 (deleted in the send snapshot), which does not cause any immediate and deterministic failure, because when the target inode is not found in the send snapshot, the send.c:send_utimes() function ignores it and uses the leaf region pointed to by path->slots[0], which can be any unrelated item (belonging to other inode) or it can be a region outside the leaf boundaries, if the leaf is full and path->slots[0] matches the number of items in the leaf. So we end up either successfully sending a utimes operation, which is fine and irrelevant because the old parent (inode 261) will end up being deleted later, or we end up doing an invalid memory access tha crashes the kernel. So fix this by making apply_dir_move() issue utimes operations only for parents that still exist in the send snapshot. In a separate patch we will make send_utimes() return an error (-ENOENT) if the given inode does not exists in the send snapshot. Signed-off-by: Robbie Ko <[email protected]> Signed-off-by: Filipe Manana <[email protected]> [Rewrote change log to be more detailed and better organized] Signed-off-by: Filipe Manana <[email protected]>
1 parent 443f9d2 commit 764433a

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

fs/btrfs/send.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3270,8 +3270,18 @@ static int apply_dir_move(struct send_ctx *sctx, struct pending_dir_move *pm)
32703270
* and old parent(s).
32713271
*/
32723272
list_for_each_entry(cur, &pm->update_refs, list) {
3273-
if (cur->dir == rmdir_ino)
3273+
/*
3274+
* The parent inode might have been deleted in the send snapshot
3275+
*/
3276+
ret = get_inode_info(sctx->send_root, cur->dir, NULL,
3277+
NULL, NULL, NULL, NULL, NULL);
3278+
if (ret == -ENOENT) {
3279+
ret = 0;
32743280
continue;
3281+
}
3282+
if (ret < 0)
3283+
goto out;
3284+
32753285
ret = send_utimes(sctx, cur->dir, cur->dir_gen);
32763286
if (ret < 0)
32773287
goto out;

0 commit comments

Comments
 (0)