mirror of
				https://github.com/torvalds/linux.git
				synced 2025-10-31 16:48:26 +02:00 
			
		
		
		
	btrfs: extent-tree: kill BUG_ON() in __btrfs_free_extent()
__btrfs_free_extent() is doing two things: 1. Reduce the refs number of an extent backref Either it's an inline extent backref (inside EXTENT/METADATA item) or a keyed extent backref (SHARED_* item). We only need to locate that backref line, either reduce the number or remove the backref line completely. 2. Update the refs count in EXTENT/METADATA_ITEM During step 1), we will try to locate the EXTENT/METADATA_ITEM without triggering another btrfs_search_slot() as fast path. Only when we fail to locate that item, we will trigger another btrfs_search_slot() to get that EXTENT/METADATA_ITEM after we updated/deleted the backref line. And we have a lot of strict checks on things like refs_to_drop against extent refs and special case checks for single ref extents. There are 7 BUG_ON()s, although they're doing correct checks, they can be triggered by crafted images. This patch improves the function: - Introduce two examples to show what __btrfs_free_extent() is doing One inline backref case and one keyed case. Should cover most cases. - Kill all BUG_ON()s with proper error message and optional leaf dump - Add comment to show the overall flow Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202819 [ The report triggers one BUG_ON() in __btrfs_free_extent() ] Reviewed-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
		
							parent
							
								
									f98b6215d7
								
							
						
					
					
						commit
						1c2a07f598
					
				
					 1 changed files with 147 additions and 13 deletions
				
			
		|  | @ -2934,6 +2934,65 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | /*
 | ||||||
|  |  * Drop one or more refs of @node. | ||||||
|  |  * | ||||||
|  |  * 1. Locate the extent refs. | ||||||
|  |  *    It's either inline in EXTENT/METADATA_ITEM or in keyed SHARED_* item. | ||||||
|  |  *    Locate it, then reduce the refs number or remove the ref line completely. | ||||||
|  |  * | ||||||
|  |  * 2. Update the refs count in EXTENT/METADATA_ITEM | ||||||
|  |  * | ||||||
|  |  * Inline backref case: | ||||||
|  |  * | ||||||
|  |  * in extent tree we have: | ||||||
|  |  * | ||||||
|  |  * 	item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82 | ||||||
|  |  *		refs 2 gen 6 flags DATA | ||||||
|  |  *		extent data backref root FS_TREE objectid 258 offset 0 count 1 | ||||||
|  |  *		extent data backref root FS_TREE objectid 257 offset 0 count 1 | ||||||
|  |  * | ||||||
|  |  * This function gets called with: | ||||||
|  |  * | ||||||
|  |  *    node->bytenr = 13631488 | ||||||
|  |  *    node->num_bytes = 1048576 | ||||||
|  |  *    root_objectid = FS_TREE | ||||||
|  |  *    owner_objectid = 257 | ||||||
|  |  *    owner_offset = 0 | ||||||
|  |  *    refs_to_drop = 1 | ||||||
|  |  * | ||||||
|  |  * Then we should get some like: | ||||||
|  |  * | ||||||
|  |  * 	item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82 | ||||||
|  |  *		refs 1 gen 6 flags DATA | ||||||
|  |  *		extent data backref root FS_TREE objectid 258 offset 0 count 1 | ||||||
|  |  * | ||||||
|  |  * Keyed backref case: | ||||||
|  |  * | ||||||
|  |  * in extent tree we have: | ||||||
|  |  * | ||||||
|  |  *	item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24 | ||||||
|  |  *		refs 754 gen 6 flags DATA | ||||||
|  |  *	[...] | ||||||
|  |  *	item 2 key (13631488 EXTENT_DATA_REF <HASH>) itemoff 3915 itemsize 28 | ||||||
|  |  *		extent data backref root FS_TREE objectid 866 offset 0 count 1 | ||||||
|  |  * | ||||||
|  |  * This function get called with: | ||||||
|  |  * | ||||||
|  |  *    node->bytenr = 13631488 | ||||||
|  |  *    node->num_bytes = 1048576 | ||||||
|  |  *    root_objectid = FS_TREE | ||||||
|  |  *    owner_objectid = 866 | ||||||
|  |  *    owner_offset = 0 | ||||||
|  |  *    refs_to_drop = 1 | ||||||
|  |  * | ||||||
|  |  * Then we should get some like: | ||||||
|  |  * | ||||||
|  |  *	item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24 | ||||||
|  |  *		refs 753 gen 6 flags DATA | ||||||
|  |  * | ||||||
|  |  * And that (13631488 EXTENT_DATA_REF <HASH>) gets removed. | ||||||
|  |  */ | ||||||
| static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 			       struct btrfs_delayed_ref_node *node, u64 parent, | 			       struct btrfs_delayed_ref_node *node, u64 parent, | ||||||
| 			       u64 root_objectid, u64 owner_objectid, | 			       u64 root_objectid, u64 owner_objectid, | ||||||
|  | @ -2966,7 +3025,15 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 	path->leave_spinning = 1; | 	path->leave_spinning = 1; | ||||||
| 
 | 
 | ||||||
| 	is_data = owner_objectid >= BTRFS_FIRST_FREE_OBJECTID; | 	is_data = owner_objectid >= BTRFS_FIRST_FREE_OBJECTID; | ||||||
| 	BUG_ON(!is_data && refs_to_drop != 1); | 
 | ||||||
|  | 	if (!is_data && refs_to_drop != 1) { | ||||||
|  | 		btrfs_crit(info, | ||||||
|  | "invalid refs_to_drop, dropping more than 1 refs for tree block %llu refs_to_drop %u", | ||||||
|  | 			   node->bytenr, refs_to_drop); | ||||||
|  | 		ret = -EINVAL; | ||||||
|  | 		btrfs_abort_transaction(trans, ret); | ||||||
|  | 		goto out; | ||||||
|  | 	} | ||||||
| 
 | 
 | ||||||
| 	if (is_data) | 	if (is_data) | ||||||
| 		skinny_metadata = false; | 		skinny_metadata = false; | ||||||
|  | @ -2975,6 +3042,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 				    parent, root_objectid, owner_objectid, | 				    parent, root_objectid, owner_objectid, | ||||||
| 				    owner_offset); | 				    owner_offset); | ||||||
| 	if (ret == 0) { | 	if (ret == 0) { | ||||||
|  | 		/*
 | ||||||
|  | 		 * Either the inline backref or the SHARED_DATA_REF/ | ||||||
|  | 		 * SHARED_BLOCK_REF is found | ||||||
|  | 		 * | ||||||
|  | 		 * Here is a quick path to locate EXTENT/METADATA_ITEM. | ||||||
|  | 		 * It's possible the EXTENT/METADATA_ITEM is near current slot. | ||||||
|  | 		 */ | ||||||
| 		extent_slot = path->slots[0]; | 		extent_slot = path->slots[0]; | ||||||
| 		while (extent_slot >= 0) { | 		while (extent_slot >= 0) { | ||||||
| 			btrfs_item_key_to_cpu(path->nodes[0], &key, | 			btrfs_item_key_to_cpu(path->nodes[0], &key, | ||||||
|  | @ -2991,13 +3065,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 				found_extent = 1; | 				found_extent = 1; | ||||||
| 				break; | 				break; | ||||||
| 			} | 			} | ||||||
|  | 
 | ||||||
|  | 			/* Quick path didn't find the EXTEMT/METADATA_ITEM */ | ||||||
| 			if (path->slots[0] - extent_slot > 5) | 			if (path->slots[0] - extent_slot > 5) | ||||||
| 				break; | 				break; | ||||||
| 			extent_slot--; | 			extent_slot--; | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| 		if (!found_extent) { | 		if (!found_extent) { | ||||||
| 			BUG_ON(iref); | 			if (iref) { | ||||||
|  | 				btrfs_crit(info, | ||||||
|  | "invalid iref, no EXTENT/METADATA_ITEM found but has inline extent ref"); | ||||||
|  | 				btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 				goto err_dump; | ||||||
|  | 			} | ||||||
|  | 			/* Must be SHARED_* item, remove the backref first */ | ||||||
| 			ret = remove_extent_backref(trans, path, NULL, | 			ret = remove_extent_backref(trans, path, NULL, | ||||||
| 						    refs_to_drop, | 						    refs_to_drop, | ||||||
| 						    is_data, &last_ref); | 						    is_data, &last_ref); | ||||||
|  | @ -3008,6 +3090,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 			btrfs_release_path(path); | 			btrfs_release_path(path); | ||||||
| 			path->leave_spinning = 1; | 			path->leave_spinning = 1; | ||||||
| 
 | 
 | ||||||
|  | 			/* Slow path to locate EXTENT/METADATA_ITEM */ | ||||||
| 			key.objectid = bytenr; | 			key.objectid = bytenr; | ||||||
| 			key.type = BTRFS_EXTENT_ITEM_KEY; | 			key.type = BTRFS_EXTENT_ITEM_KEY; | ||||||
| 			key.offset = num_bytes; | 			key.offset = num_bytes; | ||||||
|  | @ -3082,19 +3165,26 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 	if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID && | 	if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID && | ||||||
| 	    key.type == BTRFS_EXTENT_ITEM_KEY) { | 	    key.type == BTRFS_EXTENT_ITEM_KEY) { | ||||||
| 		struct btrfs_tree_block_info *bi; | 		struct btrfs_tree_block_info *bi; | ||||||
| 		BUG_ON(item_size < sizeof(*ei) + sizeof(*bi)); | 		if (item_size < sizeof(*ei) + sizeof(*bi)) { | ||||||
|  | 			btrfs_crit(info, | ||||||
|  | "invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %lu", | ||||||
|  | 				   key.objectid, key.type, key.offset, | ||||||
|  | 				   owner_objectid, item_size, | ||||||
|  | 				   sizeof(*ei) + sizeof(*bi)); | ||||||
|  | 			btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 			goto err_dump; | ||||||
|  | 		} | ||||||
| 		bi = (struct btrfs_tree_block_info *)(ei + 1); | 		bi = (struct btrfs_tree_block_info *)(ei + 1); | ||||||
| 		WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi)); | 		WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi)); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	refs = btrfs_extent_refs(leaf, ei); | 	refs = btrfs_extent_refs(leaf, ei); | ||||||
| 	if (refs < refs_to_drop) { | 	if (refs < refs_to_drop) { | ||||||
| 		btrfs_err(info, | 		btrfs_crit(info, | ||||||
| 			  "trying to drop %d refs but we only have %Lu for bytenr %Lu", | 		"trying to drop %d refs but we only have %llu for bytenr %llu", | ||||||
| 			  refs_to_drop, refs, bytenr); | 			  refs_to_drop, refs, bytenr); | ||||||
| 		ret = -EINVAL; | 		btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
| 		btrfs_abort_transaction(trans, ret); | 		goto err_dump; | ||||||
| 		goto out; |  | ||||||
| 	} | 	} | ||||||
| 	refs -= refs_to_drop; | 	refs -= refs_to_drop; | ||||||
| 
 | 
 | ||||||
|  | @ -3106,7 +3196,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 		 * be updated by remove_extent_backref | 		 * be updated by remove_extent_backref | ||||||
| 		 */ | 		 */ | ||||||
| 		if (iref) { | 		if (iref) { | ||||||
| 			BUG_ON(!found_extent); | 			if (!found_extent) { | ||||||
|  | 				btrfs_crit(info, | ||||||
|  | "invalid iref, got inlined extent ref but no EXTENT/METADATA_ITEM found"); | ||||||
|  | 				btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 				goto err_dump; | ||||||
|  | 			} | ||||||
| 		} else { | 		} else { | ||||||
| 			btrfs_set_extent_refs(leaf, ei, refs); | 			btrfs_set_extent_refs(leaf, ei, refs); | ||||||
| 			btrfs_mark_buffer_dirty(leaf); | 			btrfs_mark_buffer_dirty(leaf); | ||||||
|  | @ -3121,13 +3216,39 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| 			} | 			} | ||||||
| 		} | 		} | ||||||
| 	} else { | 	} else { | ||||||
|  | 		/* In this branch refs == 1 */ | ||||||
| 		if (found_extent) { | 		if (found_extent) { | ||||||
| 			BUG_ON(is_data && refs_to_drop != | 			if (is_data && refs_to_drop != | ||||||
| 			       extent_data_ref_count(path, iref)); | 			    extent_data_ref_count(path, iref)) { | ||||||
|  | 				btrfs_crit(info, | ||||||
|  | 		"invalid refs_to_drop, current refs %u refs_to_drop %u", | ||||||
|  | 					   extent_data_ref_count(path, iref), | ||||||
|  | 					   refs_to_drop); | ||||||
|  | 				btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 				goto err_dump; | ||||||
|  | 			} | ||||||
| 			if (iref) { | 			if (iref) { | ||||||
| 				BUG_ON(path->slots[0] != extent_slot); | 				if (path->slots[0] != extent_slot) { | ||||||
|  | 					btrfs_crit(info, | ||||||
|  | "invalid iref, extent item key (%llu %u %llu) doesn't have wanted iref", | ||||||
|  | 						   key.objectid, key.type, | ||||||
|  | 						   key.offset); | ||||||
|  | 					btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 					goto err_dump; | ||||||
|  | 				} | ||||||
| 			} else { | 			} else { | ||||||
| 				BUG_ON(path->slots[0] != extent_slot + 1); | 				/*
 | ||||||
|  | 				 * No inline ref, we must be at SHARED_* item, | ||||||
|  | 				 * And it's single ref, it must be: | ||||||
|  | 				 * |	extent_slot	  ||extent_slot + 1| | ||||||
|  | 				 * [ EXTENT/METADATA_ITEM ][ SHARED_* ITEM ] | ||||||
|  | 				 */ | ||||||
|  | 				if (path->slots[0] != extent_slot + 1) { | ||||||
|  | 					btrfs_crit(info, | ||||||
|  | 	"invalid SHARED_* item, previous item is not EXTENT/METADATA_ITEM"); | ||||||
|  | 					btrfs_abort_transaction(trans, -EUCLEAN); | ||||||
|  | 					goto err_dump; | ||||||
|  | 				} | ||||||
| 				path->slots[0] = extent_slot; | 				path->slots[0] = extent_slot; | ||||||
| 				num_to_del = 2; | 				num_to_del = 2; | ||||||
| 			} | 			} | ||||||
|  | @ -3168,6 +3289,19 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, | ||||||
| out: | out: | ||||||
| 	btrfs_free_path(path); | 	btrfs_free_path(path); | ||||||
| 	return ret; | 	return ret; | ||||||
|  | err_dump: | ||||||
|  | 	/*
 | ||||||
|  | 	 * Leaf dump can take up a lot of log buffer, so we only do full leaf | ||||||
|  | 	 * dump for debug build. | ||||||
|  | 	 */ | ||||||
|  | 	if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) { | ||||||
|  | 		btrfs_crit(info, "path->slots[0]=%d extent_slot=%d", | ||||||
|  | 			   path->slots[0], extent_slot); | ||||||
|  | 		btrfs_print_leaf(path->nodes[0]); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	btrfs_free_path(path); | ||||||
|  | 	return -EUCLEAN; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /*
 | /*
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Qu Wenruo
						Qu Wenruo