On Fri, 15 Jul 2011, Alex Elder wrote:
> On Tue, 2011-06-28 at 16:26 +0200, Lukas Czerner wrote:
> > When getting an inode tree pointer from an array inode_tree_ptrs, we
> > should check if agno, which is used as a pointer to the array, lives
> > within the file system, because if it is not, we can end up touching
> > uninitialized memory. This may happen if we have corrupted directory
> > entry.
> >
> > This commit fixes it by passing xfs_mount to affected functions and
> > checking if agno really is inside the file system.
> >
> > This solves Red Hat bug #694706
> >
> > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
>
> OK, it looks like there are basically four changes
> here--adding the mount point argument to four functions
> and using it to verify (or assert the validity of) the
> an AG number. The rest of the changes are simply the
> rippling-back effect of adding the mount point.
>
> The change looks good to me. If nobody else objects to
> the change I will commit this for you.
>
> Reviewed-by: Alex Elder <aelder@xxxxxxx>
Thanks Alex!
I still can not see it in the git though, so what is the status of this
patch ?
Thanks!
-Lukas
>
> . . .
>
> > diff --git a/repair/incore.h b/repair/incore.h
> > index 99853fb..5e3d95d 100644
> > --- a/repair/incore.h
> > +++ b/repair/incore.h
>
> . . .
>
> > @@ -316,12 +317,17 @@ findfirst_inode_rec(xfs_agnumber_t agno)
> > return((ino_tree_node_t *) inode_tree_ptrs[agno]->avl_firstino);
> > }
> > static inline ino_tree_node_t *
> > -find_inode_rec(xfs_agnumber_t agno, xfs_agino_t ino)
> > +find_inode_rec(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agino_t ino)
> > {
> > + /*
> > + * Is the AG inside the file system
> > + */
> > + if (agno >= mp->m_sb.sb_agcount)
> > + return NULL;
>
> Here is one--validate the agno using the new mp argument.
>
> > return((ino_tree_node_t *)
> > avl_findrange(inode_tree_ptrs[agno], ino));
> > }
> > -void find_inode_rec_range(xfs_agnumber_t agno,
> > +void find_inode_rec_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> > xfs_agino_t start_ino, xfs_agino_t end_ino,
> > ino_tree_node_t **first, ino_tree_node_t **last);
> >
>
> . . .
>
> > diff --git a/repair/incore_ino.c b/repair/incore_ino.c
> > index febe0c9..7827ff5 100644
> > --- a/repair/incore_ino.c
> > +++ b/repair/incore_ino.c
> > @@ -418,9 +418,11 @@ add_inode_uncertain(xfs_mount_t *mp, xfs_ino_t ino, int free)
> > * pull the indicated inode record out of the uncertain inode tree
> > */
> > void
> > -get_uncertain_inode_rec(xfs_agnumber_t agno, ino_tree_node_t *ino_rec)
> > +get_uncertain_inode_rec(struct xfs_mount *mp, xfs_agnumber_t agno,
> > + ino_tree_node_t *ino_rec)
> > {
> > ASSERT(inode_tree_ptrs != NULL);
> > + ASSERT(agno < mp->m_sb.sb_agcount);
>
> Here is the second.
>
> > ASSERT(inode_tree_ptrs[agno] != NULL);
> >
> > avl_delete(inode_uncertain_tree_ptrs[agno], &ino_rec->avl_node);
>
> . . .
>
> > @@ -495,9 +497,10 @@ add_inode(xfs_agnumber_t agno, xfs_agino_t ino)
> > * pull the indicated inode record out of the inode tree
> > */
> > void
> > -get_inode_rec(xfs_agnumber_t agno, ino_tree_node_t *ino_rec)
> > +get_inode_rec(struct xfs_mount *mp, xfs_agnumber_t agno, ino_tree_node_t *ino_rec)
> > {
> > ASSERT(inode_tree_ptrs != NULL);
> > + ASSERT(agno < mp->m_sb.sb_agcount);
>
> Here is the third.
>
> > ASSERT(inode_tree_ptrs[agno] != NULL);
> >
> > avl_delete(inode_tree_ptrs[agno], &ino_rec->avl_node);
>
> . . .
>
> > @@ -518,14 +521,18 @@ free_inode_rec(xfs_agnumber_t agno, ino_tree_node_t *ino_rec)
> > }
> >
> > void
> > -find_inode_rec_range(xfs_agnumber_t agno, xfs_agino_t start_ino,
> > - xfs_agino_t end_ino, ino_tree_node_t **first,
> > - ino_tree_node_t **last)
> > +find_inode_rec_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> > + xfs_agino_t start_ino, xfs_agino_t end_ino,
> > + ino_tree_node_t **first, ino_tree_node_t **last)
> > {
> > *first = *last = NULL;
> >
> > - avl_findranges(inode_tree_ptrs[agno], start_ino,
> > - end_ino, (avlnode_t **) first, (avlnode_t **) last);
> > + /*
> > + * Is the AG inside the file system ?
> > + */
> > + if (agno < mp->m_sb.sb_agcount)
> > + avl_findranges(inode_tree_ptrs[agno], start_ino,
> > + end_ino, (avlnode_t **) first, (avlnode_t **) last);
>
> And here is the fourth.
>
> > }
> >
> > /*
>
> . . .
>
>
>
>
--
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
