On 08/14/13 19:33, Dave Chinner wrote:
On Wed, Aug 14, 2013 at 08:33:03AM -0500, Mark Tinguely wrote:
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) {
BTW, Mark, you're mailer is doing weird things to whitespace in code
when it's quoting quoted code.
Thanks.
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.
It's the same code as in the kernel - if is that broken that it
can't tell it didn't allocate a real inode, then we've got bigger
problems. We design the code to return an error when it fails so we
don't have to robustly check every possible error condition at every
call site. So really the only check needed is "if (!irec) {...}"
Cheers,
Dave.
The inode number check came from find_inode_rec(ag, agino)
(repair/incore.h). A bad inode number will also return a NULL irec.
--Mark.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
