| Message ID | 1473233938-21560-1-git-send-email-kernel@kyup.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 07, 2016 at 10:38:58AM +0300, Nikolay Borisov wrote: > btrfs_uuid_iter_rem is able to return -ENOENT, however this condition > is not handled in btrfs_uuid_tree_iterate which can lead to calling > btrfs_next_item with freed path argument, leading to a null pointer > dereference. Fix it by redoing the search but with an incremented > objectid so we don't loop over the same key. > > Signed-off-by: Nikolay Borisov <kernel@kyup.com> > Suggested-by: Chris Mason <clm@fb.com> > Link: https://lkml.kernel.org/r/57A473B0.2040203@kyup.com I'll queue the patch for 4.9, thanks. -- 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
On 09/19/2016 02:13 PM, David Sterba wrote: > On Wed, Sep 07, 2016 at 10:38:58AM +0300, Nikolay Borisov wrote: >> btrfs_uuid_iter_rem is able to return -ENOENT, however this condition >> is not handled in btrfs_uuid_tree_iterate which can lead to calling >> btrfs_next_item with freed path argument, leading to a null pointer >> dereference. Fix it by redoing the search but with an incremented >> objectid so we don't loop over the same key. >> >> Signed-off-by: Nikolay Borisov <kernel@kyup.com> >> Suggested-by: Chris Mason <clm@fb.com> >> Link: https://lkml.kernel.org/r/57A473B0.2040203@kyup.com > > I'll queue the patch for 4.9, thanks. > Not having a good test for this kept me from trying the patch cold. I think bumping the objectid will end up missing items. We know its returning -ENOENT, so it should in theory be enough to just goto again_search_slot, assuming that we just raced with the deletion. -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
On Mon, Sep 19, 2016 at 02:49:41PM -0400, Chris Mason wrote: > On 09/19/2016 02:13 PM, David Sterba wrote: > > On Wed, Sep 07, 2016 at 10:38:58AM +0300, Nikolay Borisov wrote: > >> btrfs_uuid_iter_rem is able to return -ENOENT, however this condition > >> is not handled in btrfs_uuid_tree_iterate which can lead to calling > >> btrfs_next_item with freed path argument, leading to a null pointer > >> dereference. Fix it by redoing the search but with an incremented > >> objectid so we don't loop over the same key. > >> > >> Signed-off-by: Nikolay Borisov <kernel@kyup.com> > >> Suggested-by: Chris Mason <clm@fb.com> > >> Link: https://lkml.kernel.org/r/57A473B0.2040203@kyup.com > > > > I'll queue the patch for 4.9, thanks. > > > > Not having a good test for this kept me from trying the patch cold. I > think bumping the objectid will end up missing items. Ok, so I can keep it in the branches that are not for the upcoming merges but still in for-next. -- 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
[Resend due to gmail mobile interface defaulting to html layout] >> >> We know its returning -ENOENT, so it should in theory be enough to just >> goto again_search_slot, assuming that we just raced with the deletion. > > > I will apply this on the machine which are exhibitting problems and will > report whether it rectified the situation. i bump the objectid wince this is > what you suggested. i can also try without it. >> >> >> -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/uuid-tree.c b/fs/btrfs/uuid-tree.c index 778282944530..6e5b3866a65c 100644 --- a/fs/btrfs/uuid-tree.c +++ b/fs/btrfs/uuid-tree.c @@ -329,8 +329,12 @@ again_search_slot: * entry per UUID exists. */ goto again_search_slot; - } - if (ret < 0 && ret != -ENOENT) + } else if (ret == -ENOENT) { + key.type = 0; + key.offset = 0; + key.objectid++; + goto again_search_slot; + } else if (ret < 0) goto out; } item_size -= sizeof(subid_le);
btrfs_uuid_iter_rem is able to return -ENOENT, however this condition is not handled in btrfs_uuid_tree_iterate which can lead to calling btrfs_next_item with freed path argument, leading to a null pointer dereference. Fix it by redoing the search but with an incremented objectid so we don't loop over the same key. Signed-off-by: Nikolay Borisov <kernel@kyup.com> Suggested-by: Chris Mason <clm@fb.com> Link: https://lkml.kernel.org/r/57A473B0.2040203@kyup.com --- fs/btrfs/uuid-tree.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) Hello Chris, Since I keep getting those crashes I (hopefully correctly) implemented your suggestion of redoing the search with an incremented key so we don't end up in a loop. Does that look correct?