Re: [PATCH] xfsprogs: fix inode crash in xfs_repair

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/14/13 01:40, Dave Chinner wrote:
On Tue, Aug 13, 2013 at 05:13:31PM -0500, Mark Tinguely wrote:
Adding the lost+found in phase 6 could allocate an inode from
a new inode chunk. That newly created chunk was not around in
the scan phase, and is not in the avl tree which will result
in a NULL dereference.

This patch adds the newly created inode chunk and inodes as if
found in the scan phase.

Metadata dump available for future tests.

Signed-off-by: Mark Tinguely<tinguely@xxxxxxx>
---
  repair/incore_ino.c |    2 +-
  repair/phase6.c     |   15 +++++++++++++++
  2 files changed, 16 insertions(+), 1 deletion(-)

Index: b/repair/incore_ino.c
===================================================================
--- a/repair/incore_ino.c
+++ b/repair/incore_ino.c
@@ -700,7 +700,7 @@ get_inode_parent(ino_tree_node_t *irec,
  	return(0LL);
  }

-static void
+void
  alloc_ex_data(ino_tree_node_t *irec)
  {
  	parent_list_t 	*ptbl;
Index: b/repair/phase6.c
===================================================================
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -930,6 +930,21 @@ mk_orphanage(xfs_mount_t *mp)
  	irec = find_inode_rec(mp,
  			XFS_INO_TO_AGNO(mp, ino),
  			XFS_INO_TO_AGINO(mp, ino));
+
+	if (irec == NULL&&  XFS_INO_TO_AGNO(mp, ino)<  mp->m_sb.sb_agcount&&
+	    ip != NULL&&  ip->i_d.di_magic == XFS_DINODE_MAGIC) {

I don't understand this check.

We've already dereferenced ip several lines above to increment the
link count and get the inode number stored in ino, so the ip != NULL
is unnecessary.

We've just allocated the inode, so why would the magic number be
wrong? And why would the inode number lie in a non-existent
allocation group?


just being being paranoid.

+		/*
+		 * add the newly allocated inode chunk to the avl tree.
+		 */

I can see from the code we are allocating and irec, inserting it
into the AVL tree and marking all the inodes in the chunk as free.
The comment should explain *why* we need to do this.

Cheers,

Dave.

--Mark.

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs



[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux