All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramon de Carvalho Valle <ramon@risesecurity.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com, hch@lst.de, linux-kernel@vger.kernel.org,
	mszeredi@novell.com
Subject: Re: [PATCH 1/1] XFS: __xfs_get_blocks check pointer to the target device
Date: Mon, 03 Aug 2009 23:00:41 -0300	[thread overview]
Message-ID: <1249351241.7513.18.camel@logos> (raw)
In-Reply-To: <20090803214929.GB3167@infradead.org>


[-- Attachment #1.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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

WARNING: multiple messages have this Message-ID (diff)
From: Ramon de Carvalho Valle <ramon@risesecurity.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-kernel@vger.kernel.org, mszeredi@novell.com, hch@lst.de,
	xfs@oss.sgi.com
Subject: Re: [PATCH 1/1] XFS: __xfs_get_blocks check pointer to the target device
Date: Mon, 03 Aug 2009 23:00:41 -0300	[thread overview]
Message-ID: <1249351241.7513.18.camel@logos> (raw)
In-Reply-To: <20090803214929.GB3167@infradead.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 --]

  reply	other threads:[~2009-08-04  2:00 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-03 20:03 [PATCH 1/1] XFS: __xfs_get_blocks check pointer to the target device Ramon de Carvalho Valle
2009-08-03 20:03 ` Ramon de Carvalho Valle
2009-08-03 21:49 ` Christoph Hellwig
2009-08-03 21:49   ` Christoph Hellwig
2009-08-04  2:00   ` Ramon de Carvalho Valle [this message]
2009-08-04  2:00     ` Ramon de Carvalho Valle
2009-08-04 14:31     ` Christoph Hellwig
2009-08-04 14:31       ` Christoph Hellwig
2009-08-04 16:25     ` Eric Sandeen
2009-08-04 16:25       ` Eric Sandeen
2009-08-04 18:50       ` Ramon de Carvalho Valle
2009-08-04 18:50         ` Ramon de Carvalho Valle
2009-08-04 18:51       ` [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check Ramon de Carvalho Valle
2009-08-04 18:51         ` Ramon de Carvalho Valle
2009-08-04 19:11         ` Eric Sandeen
2009-08-04 19:11           ` Eric Sandeen
2009-08-05  3:55           ` Ramon de Carvalho Valle
2009-08-05  3:55             ` Ramon de Carvalho Valle
2009-08-05  4:15             ` Eric Sandeen
2009-08-05  4:15               ` Eric Sandeen
2009-08-05 13:21               ` Ramon de Carvalho Valle
2009-08-05 13:21                 ` Ramon de Carvalho Valle
2009-08-05 21:53               ` Felix Blyakher
2009-08-05 21:53                 ` Felix Blyakher
2009-08-05 15:17         ` Christoph Hellwig
2009-08-05 15:17           ` Christoph Hellwig
2009-08-05 16:34           ` Ramon de Carvalho Valle
2009-08-05 16:34             ` Ramon de Carvalho Valle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1249351241.7513.18.camel@logos \
    --to=ramon@risesecurity.org \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@novell.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.