[PATCH] sanity check attr fork size

[PATCH] sanity check attr fork size

[Christoph Hellwig]

Recently we have quite a few kerneloops reports about dereferencing a NULL
if_data in the attribute fork.  From looking over the code this can only
happen if we pass a 0 size argument to xfs_iformat_local.  This implies some
sort of corruption and in fact the only mailinglist report about this from
earlier this year was after a powerfail presumably on a system with write
cache and without barriers.

Add a quick sanity check for the attr fork size in xfs_iformat to catch
these early and without an oops.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/xfs/xfs_inode.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_inode.c	2009-01-04 17:35:11.955671640 +0100
+++ linux-2.6/fs/xfs/xfs_inode.c	2009-01-04 17:38:04.643673527 +0100
@@ -424,6 +424,19 @@ xfs_iformat(
 	case XFS_DINODE_FMT_LOCAL:
 		atp = (xfs_attr_shortform_t *)XFS_DFORK_APTR(dip);
 		size = be16_to_cpu(atp->hdr.totsize);
+
+		if (unlikely(size < sizeof(struct xfs_attr_sf_hdr))) {
+			xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
+				"corrupt inode %Lu "
+				"(bad attr fork size %Ld).",
+				(unsigned long long) ip->i_ino,
+				(long long) size);
+			XFS_CORRUPTION_ERROR("xfs_iformat(8)",
+					     XFS_ERRLEVEL_LOW,
+					     ip->i_mount, dip);
+			return XFS_ERROR(EFSCORRUPTED);
+		}
+
 		error = xfs_iformat_local(ip, dip, XFS_ATTR_FORK, size);
 		break;
 	case XFS_DINODE_FMT_EXTENTS:

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

Re: [PATCH] sanity check attr fork size

[Dave Chinner]

On Sun, Jan 04, 2009 at 11:42:11AM -0500, Christoph Hellwig wrote:
> Recently we have quite a few kerneloops reports about dereferencing a NULL
> if_data in the attribute fork.  From looking over the code this can only
> happen if we pass a 0 size argument to xfs_iformat_local.  This implies some
> sort of corruption and in fact the only mailinglist report about this from
> earlier this year was after a powerfail presumably on a system with write
> cache and without barriers.
> 
> Add a quick sanity check for the attr fork size in xfs_iformat to catch
> these early and without an oops.

Looks sane to me.

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

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