Re: [PATCH 1/1] XFS: __xfs_get_blocks check pointer to the target device

Christoph Hellwig
2009-08-04T14:30:54Z

On Mon, Aug 03, 2009 at 11:00:41PM -0300, Ramon de Carvalho Valle wrote:
> On Mon, 2009-08-03 at 17:49 -0400, Christoph Hellwig wrote:
> > On Mon, Aug 03, 2009 at 05:03:28PM -0300, Ramon de Carvalho Valle wrote:
> > > The __xfs_get_blocks function does not check if the pointer to the target
> > > device is valid before dereferencing it.
> >
> > It should never be zero. It's set by xfs_imap_to_bmap to either
> > mp->m_ddev_targp which is always allocated, or to mp->m_rtdev_targp
> > which is always allocated if we have a realtime device, and
> > XFS_IS_REALTIME_INODE should only be true in that case.
> >
>
> While testing XFS code with a modified version of fsfuzzer on SLES 10
> SP3 (Kernel 2.6.16.60-0.49.3.ramon-ppc64), I came across the following
> Oops:

So you actually introduced the RT flag in the inode on a filesystem
where it can't happen software-wise.

I'd rather deal with this early on to protect the invariant that's
guaranteed inside the xfs code.

Something like the following untested patch:

Index: linux-2.6/fs/xfs/xfs_inode.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_inode.c 2009-08-04 16:19:40.778532419 +0200
+++ linux-2.6/fs/xfs/xfs_inode.c 2009-08-04 16:28:06.352533058 +0200
@@ -343,6 +343,16 @@ xfs_iformat(
return XFS_ERROR(EFSCORRUPTED);
}

+ if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
+ !ip->i_mount->m_rtdev_targp)) {
+ xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
+ "corrupt dinode %Lu, has realtime flag set.",
+ ip->i_ino);
+ XFS_CORRUPTION_ERROR("xfs_iformat(realtime)",
+ XFS_ERRLEVEL_LOW, ip->i_mount, dip);
+ return XFS_ERROR(EFSCORRUPTED);
+ }
+
switch (ip->i_d.di_mode & S_IFMT) {