From 418553acc64567ebc95122e28b07657526c92923 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Wed, 22 Mar 2023 17:25:54 -0400 Subject: btrfs: Consider the generation when checking if a node owner is OK --- lib/btrfs/btrfstree/readnode.go | 34 ++++++++++++++++++++------------- lib/btrfs/io2_lv.go | 3 +-- lib/btrfs/io3_btree.go | 37 ++++++++++++++++++++++-------------- lib/btrfsutil/old_rebuilt_forrest.go | 9 +++++---- lib/btrfsutil/rebuilt_readitem.go | 9 +++++---- 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/lib/btrfs/btrfstree/readnode.go b/lib/btrfs/btrfstree/readnode.go index 7cc42f5..ac82c62 100644 --- a/lib/btrfs/btrfstree/readnode.go +++ b/lib/btrfs/btrfstree/readnode.go @@ -20,13 +20,13 @@ type NodeFile interface { // ParentTree, given a tree ID, returns that tree's parent // tree, if it has one. // - // - non-zero, true : the parent tree ID + // - non-zero, ?, true : the parent tree ID // - // - 0, true : the tree does not have a parent + // - 0, 0, true : the tree does not have a parent // - // - any, false : the tree's parent information could not be + // - ?, ?, false : the tree's parent information could not be // looked up - ParentTree(btrfsprim.ObjID) (btrfsprim.ObjID, bool) + ParentTree(btrfsprim.ObjID) (btrfsprim.ObjID, btrfsprim.Generation, bool) } // FSReadNode is a utility function to help with implementing the @@ -40,25 +40,33 @@ func FSReadNode( return nil, fmt.Errorf("btrfs.FS.ReadNode: %w", err) } - var treeParents []btrfsprim.ObjID - checkOwner := func(owner btrfsprim.ObjID, _ btrfsprim.Generation) error { - exp := path.Node(-1).FromTree + checkOwner := func(owner btrfsprim.ObjID, gen btrfsprim.Generation) error { + var treeParents []btrfsprim.ObjID + + tree := path.Node(-1).FromTree for { - if owner == exp { + if owner == tree { + // OK! return nil } - treeParents = append(treeParents, exp) - var ok bool - exp, ok = fs.ParentTree(exp) - if !ok { + + treeParents = append(treeParents, tree) + parent, parentGen, parentOK := fs.ParentTree(tree) + if !parentOK { // Failed look up parent info; fail open. return nil } - if exp == 0 { + + if parent == 0 { // End of the line. return fmt.Errorf("expected owner in %v but claims to have owner=%v", treeParents, owner) } + if gen > parentGen { + return fmt.Errorf("claimed owner=%v might be acceptable in this tree (if generation<=%v) but not with claimed generation=%v", + owner, parentGen, gen) + } + tree = parent } } diff --git a/lib/btrfs/io2_lv.go b/lib/btrfs/io2_lv.go index 9f4e53f..03b2107 100644 --- a/lib/btrfs/io2_lv.go +++ b/lib/btrfs/io2_lv.go @@ -27,9 +27,8 @@ type FS struct { cacheSuperblocks []*diskio.Ref[btrfsvol.PhysicalAddr, btrfstree.Superblock] cacheSuperblock *btrfstree.Superblock - cacheObjID2UUID map[btrfsprim.ObjID]btrfsprim.UUID + cacheObjID2All map[btrfsprim.ObjID]treeInfo cacheUUID2ObjID map[btrfsprim.UUID]btrfsprim.ObjID - cacheTreeParent map[btrfsprim.ObjID]btrfsprim.UUID } var _ diskio.File[btrfsvol.LogicalAddr] = (*FS)(nil) diff --git a/lib/btrfs/io3_btree.go b/lib/btrfs/io3_btree.go index 6df88f5..b60f54a 100644 --- a/lib/btrfs/io3_btree.go +++ b/lib/btrfs/io3_btree.go @@ -25,13 +25,18 @@ var _ btrfstree.NodeSource = (*FS)(nil) // btrfstree.NodeFile ////////////////////////////////////////////////////////// +type treeInfo struct { + UUID btrfsprim.UUID + ParentUUID btrfsprim.UUID + ParentGen btrfsprim.Generation +} + func (fs *FS) populateTreeUUIDs(ctx context.Context) { - if fs.cacheObjID2UUID != nil && fs.cacheUUID2ObjID != nil && fs.cacheTreeParent != nil { + if fs.cacheObjID2All != nil && fs.cacheUUID2ObjID != nil { return } - fs.cacheObjID2UUID = make(map[btrfsprim.ObjID]btrfsprim.UUID) + fs.cacheObjID2All = make(map[btrfsprim.ObjID]treeInfo) fs.cacheUUID2ObjID = make(map[btrfsprim.UUID]btrfsprim.ObjID) - fs.cacheTreeParent = make(map[btrfsprim.ObjID]btrfsprim.UUID) fs.TreeWalk(ctx, btrfsprim.ROOT_TREE_OBJECTID, func(err *btrfstree.TreeError) { // do nothing @@ -42,8 +47,11 @@ func (fs *FS) populateTreeUUIDs(ctx context.Context) { if !ok { return } - fs.cacheObjID2UUID[item.Key.ObjectID] = itemBody.UUID - fs.cacheTreeParent[item.Key.ObjectID] = itemBody.ParentUUID + fs.cacheObjID2All[item.Key.ObjectID] = treeInfo{ + UUID: itemBody.UUID, + ParentUUID: itemBody.ParentUUID, + ParentGen: btrfsprim.Generation(item.Key.Offset), + } fs.cacheUUID2ObjID[itemBody.UUID] = item.Key.ObjectID }, }, @@ -51,27 +59,28 @@ func (fs *FS) populateTreeUUIDs(ctx context.Context) { } // ParentTree implements btrfstree.NodeFile. -func (fs *FS) ParentTree(tree btrfsprim.ObjID) (btrfsprim.ObjID, bool) { +func (fs *FS) ParentTree(tree btrfsprim.ObjID) (btrfsprim.ObjID, btrfsprim.Generation, bool) { if tree < btrfsprim.FIRST_FREE_OBJECTID || tree > btrfsprim.LAST_FREE_OBJECTID { // no parent - return 0, true + return 0, 0, true } fs.populateTreeUUIDs(context.TODO()) - parentUUID, ok := fs.cacheTreeParent[tree] + + all, ok := fs.cacheObjID2All[tree] if !ok { // could not look up parent info - return 0, false + return 0, 0, false } - if parentUUID == (btrfsprim.UUID{}) { + if all.ParentUUID == (btrfsprim.UUID{}) { // no parent - return 0, true + return 0, 0, true } - parentObjID, ok := fs.cacheUUID2ObjID[parentUUID] + parentObjID, ok := fs.cacheUUID2ObjID[all.ParentUUID] if !ok { // could not look up parent info - return 0, false + return 0, 0, false } - return parentObjID, true + return parentObjID, all.ParentGen, true } var _ btrfstree.NodeFile = (*FS)(nil) diff --git a/lib/btrfsutil/old_rebuilt_forrest.go b/lib/btrfsutil/old_rebuilt_forrest.go index a3fc0e2..5b99892 100644 --- a/lib/btrfsutil/old_rebuilt_forrest.go +++ b/lib/btrfsutil/old_rebuilt_forrest.go @@ -240,10 +240,11 @@ func (bt *OldRebuiltForrest) readNode(nodeInfo nodeInfo) *btrfstree.Node { LAddr: containers.OptionalValue(nodeInfo.LAddr), Level: containers.OptionalValue(nodeInfo.Level), Generation: containers.OptionalValue(nodeInfo.Generation), - Owner: func(treeID btrfsprim.ObjID, _ btrfsprim.Generation) error { - if treeID != nodeInfo.Owner { - return fmt.Errorf("expected owner=%v but claims to have owner=%v", - nodeInfo.Owner, treeID) + Owner: func(treeID btrfsprim.ObjID, gen btrfsprim.Generation) error { + if treeID != nodeInfo.Owner || gen != nodeInfo.Generation { + return fmt.Errorf("expected owner=%v generation=%v but claims to have owner=%v generation=%v", + nodeInfo.Owner, nodeInfo.Generation, + treeID, gen) } return nil }, diff --git a/lib/btrfsutil/rebuilt_readitem.go b/lib/btrfsutil/rebuilt_readitem.go index 5de6864..03a7cdc 100644 --- a/lib/btrfsutil/rebuilt_readitem.go +++ b/lib/btrfsutil/rebuilt_readitem.go @@ -42,10 +42,11 @@ func (ts *RebuiltForrest) readNode(ctx context.Context, laddr btrfsvol.LogicalAd LAddr: containers.OptionalValue(laddr), Level: containers.OptionalValue(graphInfo.Level), Generation: containers.OptionalValue(graphInfo.Generation), - Owner: func(treeID btrfsprim.ObjID, _ btrfsprim.Generation) error { - if treeID != graphInfo.Owner { - return fmt.Errorf("expected owner=%v but claims to have owner=%v", - graphInfo.Owner, treeID) + Owner: func(treeID btrfsprim.ObjID, gen btrfsprim.Generation) error { + if treeID != graphInfo.Owner || gen != graphInfo.Generation { + return fmt.Errorf("expected owner=%v generation=%v but claims to have owner=%v generation=%v", + graphInfo.Owner, graphInfo.Generation, + treeID, gen) } return nil }, -- cgit v1.1-4-g5e80