linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: reinit btree pointer on attr tree inactivation walk
@ 2017-10-06 20:07 Brian Foster
  2017-10-06 20:47 ` Darrick J. Wong
  0 siblings, 1 reply; 7+ messages in thread
From: Brian Foster @ 2017-10-06 20:07 UTC (permalink / raw)
  To: linux-xfs

xfs_attr3_root_inactive() walks the attr fork tree to invalidate the
associated blocks. xfs_attr3_node_inactive() recursively descends
from internal blocks to leaf blocks, caching block address values
along the way to revisit parent blocks, locate the next entry and
descend down that branch of the tree.

The code that attempts to reread the parent block is unsafe because
it assumes that the local xfs_da_node_entry pointer remains valid
after an xfs_trans_brelse() and re-read of the parent buffer. Under
heavy memory pressure, it is possible that the buffer has been
reclaimed and reallocated by the time the parent block is reread.
This means that 'btree' can point to an invalid memory address, lead
to a random/garbage value for child_fsb and cause the subsequent
read of the attr fork to go off the rails and return a NULL buffer
for an attr fork offset that is most likely not allocated.

Note that this problem can be manufactured by setting
XFS_ATTR_BTREE_REF to 0 to prevent LRU caching of attr buffers,
creating a file with a multi-level attr fork and removing it to
trigger inactivation.

To address this problem, reinit the node/btree pointers to the
parent buffer after it has been re-read. This ensures btree points
to a valid record and allows the walk to proceed.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

I suspect this is the cause of the NULL buf problem down in
xfs_attr_inactive(). I can manufacture an instance of that problem as
noted above. We have a customer who's hitting that problem and will
attempt to validate this fix, but there is no confirmation as of yet.
I'm posting this for review in the meantime because this seems like a
legit fix regardless of whether they are hitting this or something else.

Brian

 fs/xfs/xfs_attr_inactive.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index ebd66b1..e3a950e 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -302,6 +302,8 @@ xfs_attr3_node_inactive(
 						 &bp, XFS_ATTR_FORK);
 			if (error)
 				return error;
+			node = bp->b_addr;
+			btree = dp->d_ops->node_tree_p(node);
 			child_fsb = be32_to_cpu(btree[i + 1].before);
 			xfs_trans_brelse(*trans, bp);
 		}
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-11-07 11:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-06 20:07 [PATCH] xfs: reinit btree pointer on attr tree inactivation walk Brian Foster
2017-10-06 20:47 ` Darrick J. Wong
2017-10-07 12:14   ` Brian Foster
2017-10-11 17:11   ` Marco Benatto
2017-10-11 17:30     ` Darrick J. Wong
2017-11-06 22:26     ` Luis R. Rodriguez
2017-11-07 11:02       ` Brian Foster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).