| Message ID | 1307986264-5707-1-git-send-email-dsterba@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Excerpts from David Sterba's message of 2011-06-13 13:31:04 -0400: > smatch reported a dead code. It seems to allow wrong item size counting > in leaves, as the first for loop does not adjust the maximum number for > items that would fit in BTRFS_LEAF_DATA_SIZE, and the rest of the code > works with the wrong value. The value of 'nr' is accompanied with > accumulating total_data and total_size, which are compared to the leaf > size and probably prevent this bug to do more harm, but the errorneously > computed value of 'nr' is later used in moving existing items and lastly > for setting up the item for new data. > > The bug has a potential to silently corrupt data when leaves are near to > full, though I'm not aware of any related reports so far. btrfs_insert_some_items is actually dead code. I've just deleted it instead, but you're completely right that this is a bug. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index d840893..c4407a6 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -3433,8 +3433,8 @@ int btrfs_insert_some_items(struct btrfs_trans_handle *trans, for (i = 0; i < nr; i++) { if (total_size + data_size[i] + sizeof(struct btrfs_item) > BTRFS_LEAF_DATA_SIZE(root)) { - break; nr = i; + break; } total_data += data_size[i]; total_size += data_size[i] + sizeof(struct btrfs_item);
smatch reported a dead code. It seems to allow wrong item size counting in leaves, as the first for loop does not adjust the maximum number for items that would fit in BTRFS_LEAF_DATA_SIZE, and the rest of the code works with the wrong value. The value of 'nr' is accompanied with accumulating total_data and total_size, which are compared to the leaf size and probably prevent this bug to do more harm, but the errorneously computed value of 'nr' is later used in moving existing items and lastly for setting up the item for new data. The bug has a potential to silently corrupt data when leaves are near to full, though I'm not aware of any related reports so far. Signed-off-by: David Sterba <dsterba@suse.cz> --- I haven't tested this yet (thoug I will) because I want to let it out ASAP. fs/btrfs/ctree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)