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

[Ramon de Carvalho Valle <ramon@risesecurity.org>]

[-- Attachment #1: Type: text/plain, Size: 3609 bytes --]

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:

--
iomap.iomap_target = 0000000000000000
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=128 NUMA PSERIES LPAR 
Modules linked in: xfs_quota xfs ipv6 nfs lockd nfs_acl sunrpc apparmor loop dm_mod ehea ibmvscsic sg ipr libata firmware_class sd_mod scsi_mod
NIP: D000000000532270 LR: D000000000532268 CTR: 0000000000EEBBA0
REGS: c0000000ea2b7250 TRAP: 0300   Not tainted  (2.6.16.60-0.49.3.ramon-ppc64)
MSR: 8000000000009032 <EE,ME,IR,DR>  CR: 22244882  XER: 00000007
DAR: 0000000000000008, DSISR: 0000000040000000
TASK = c00000000f2e4d90[4369] 'run_test' THREAD: c0000000ea2b4000 CPU: 1
GPR00: D000000000532268 C0000000EA2B74D0 D000000000593F20 0000000000000029 
GPR04: 8000000000009032 0000000000000000 0000000000000000 00000000000D360A 
GPR08: 80003FBFF900000C 0000000000000000 C0000000FEFEBBE8 C0000000004F6478 
GPR12: 0000000000004000 C0000000004C3800 0000000000000005 D00000000057EC20 
GPR16: C0000000F5122480 C0000000EA7F55B0 0000000000000400 C0000000EA7F55B0 
GPR20: D00000000058A798 0000000000000000 C0000000EBF868C8 0000000000000000 
GPR24: 0000000000000000 0000000000000001 0000000000000000 0000000000000000 
GPR28: C0000000EA7F55B0 C0000000EA2B7548 D00000000058C578 C0000000EBF868C8 
NIP [D000000000532270] .__xfs_get_blocks+0x170/0x2fc [xfs]
LR [D000000000532268] .__xfs_get_blocks+0x168/0x2fc [xfs]
Call Trace:
[C0000000EA2B74D0] [D000000000532268] .__xfs_get_blocks+0x168/0x2fc [xfs] (unreliable)
[C0000000EA2B75C0] [C0000000000D8EBC] .__block_prepare_write+0x198/0x520
[C0000000EA2B76C0] [C0000000000D9818] .block_prepare_write+0x34/0x64
[C0000000EA2B7740] [D000000000531798] .xfs_vm_prepare_write+0x2c/0x44 [xfs]
[C0000000EA2B77C0] [C0000000000A2804] .generic_file_buffered_write+0x300/0x7fc
[C0000000EA2B7960] [D00000000053CB6C] .xfs_write+0x67c/0xa64 [xfs]
[C0000000EA2B7AE0] [D0000000005379B0] .xfs_file_aio_write+0x8c/0xa0 [xfs]
[C0000000EA2B7B60] [C0000000000D4518] .do_sync_write+0xd0/0x12c
[C0000000EA2B7CE0] [C0000000000D53BC] .vfs_write+0x130/0x218
[C0000000EA2B7D80] [C0000000000D55B0] .SyS_write+0x58/0xa0
[C0000000EA2B7E30] [C00000000000871C] syscall_exit+0x0/0x40
Instruction dump:
40a2fff4 38000200 7d20f8a8 7d290378 7d20f9ad 40a2fff4 48000118 e87e8058 
e8810080 4800e80d e8410028 e9210080 <e8090008> f81f0030 41920048 e81f0000 
--

I added a printk() line just before the:

	bh_result->b_bdev = iomap.iomap_target->bt_bdev;

and as we can see iomap.iomap_target is NULL.

My guess is that the XFS_DIFLAG_REALTIME flag is being set incorrectly
on the xfs inode structure, setting iomapp->iomap_target to the wrong
device pointer (probably NULL).

I don't know if this is the best place to add a check, neither if
returning -XFS_ERROR(EIO) is correct at this point. Maybe doing:

	if (iomap.iomap_target && omap.iomap_flags & IOMAP_REALTIME)
		bh_result->b_bdev = iomap.iomap_target->bt_bdev;

would be a better solution.

-Ramon

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]