Re: use-after-free in xfs_bawrite()

[Steve Lord <lord@sgi.com>]

On Sat, 2003-08-02 at 03:30, Andrew Morton wrote:
> Using Linus's current tree plus all the -mm gunk I get a fairly easy oops
> running dbench on XFS on SMP with CONFIG_DEBUG_PAGEALLOC=y:

I can't hit it for some reason, but yes, I see it. As for why xfs
and dbench don't mix so well - its a 2.6 thing, but having looked
at what the other places which wait for I/O in the kernel do, perhaps
if we made some similar changes to the xfs specific spots, things might
improve. Looks like getting into io_schedule would be a good thing
in a few strategic places.

Steve



> 
> Program received signal SIGEMT, Emulation trap.
> 0xc0282c9d in xfs_iflush (ip=0xc284a004, flags=2) at fs/xfs/pagebuf/page_buf.h:397
> 397             if (!pb || atomic_read(&pb->pb_io_remaining))
> (gdb) p pb
> $1 = (page_buf_t *) 0xc98d1004
> (gdb) p *pb
> Cannot access memory at address 0xc98d1004
> (gdb) bt
> #0  0xc0282c9d in xfs_iflush (ip=0xc284a004, flags=2) at fs/xfs/pagebuf/page_buf.h:397
> #1  0xc0283dee in xfs_inode_item_push (iip=0xc2849004) at fs/xfs/xfs_inode_item.c:882
> #2  0xc0294ddf in xfs_trans_push_ail (mp=0xceb87004, threshold_lsn=21474846993) at fs/xfs/xfs_trans_ail.c:170
> #3  0xc0287622 in xlog_grant_push_ail (mp=0xceb87004, need_bytes=492072) at fs/xfs/xfs_log.c:1390
> #4  0xc028652c in xfs_log_reserve (mp=0xceb87004, unit_bytes=157880, cnt=3, ticket=0xc9451038, client=105, 
>     flags=2) at fs/xfs/xfs_log.c:461
> #5  0xc0293afd in xfs_trans_reserve (tp=0xc9451004, blocks=41, logspace=157880, rtextents=0, flags=4, logcount=3)
>     at fs/xfs/xfs_trans.c:275
> #6  0xc029c340 in xfs_mkdir (dir_bdp=0xc0c3e024, dentry=0xca964004, vap=0xc790fec4, vpp=0xc790fec0, credp=0x0)
>     at fs/xfs/xfs_vnodeops.c:2878
> #7  0xc02a687c in linvfs_mknod (dir=0xc0c3f024, dentry=0xca964004, mode=16832, rdev=0)
>     at fs/xfs/linux/xfs_iops.c:136
> #8  0xc02a6a4f in linvfs_mkdir (dir=0xc0c3f024, dentry=0xca964004, mode=448) at fs/xfs/linux/xfs_iops.c:190
> #9  0xc016d838 in vfs_mkdir (dir=0xc0c3f024, dentry=0xca964004, mode=448) at fs/namei.c:1510
> #10 0xc016d901 in sys_mkdir (pathname=0xbffff2e7 "CLIENTS/CLIENT16/~DMTMP/SEED", mode=448) at fs/namei.c:1537
> 
> The memory at 0xc98d1004 has been unmapped.
> 
> The oops is on the xfs_iflush() -> xfs_bawrite() -> pagebuf_run_queues() path.
> 
> It looks to me like pagebuf_iostart() has called pagebuf_iorequest() which called
> _pagebuf_iodone() which called pagebuf_iodone() whuich threw away the pagebuf.
> 
> If this is vaguely correct then this part of pagebuf_iostart():
> 
> 	/* Wait for I/O if we are not an async request */
> 	if ((status == 0) && (flags & PBF_ASYNC) == 0) {
> 
> also needs attention...
> 
> 
> The below quick patch fixes it up.  But it also causes zillions of dentries
> and inodes to be leaked for some reason.  Consider it a technology
> demonstration!
> 
> XFS has waaaaay too much inlining btw ;)
> 
> Seems that dbench is not XFS's favourite benchmark.  How come?  Do I need
> more logbufs?
> 
> 
> 
>  fs/xfs/xfs_buf.h |    2 ++
>  1 files changed, 2 insertions(+)
> 
> diff -puN fs/xfs/xfs_buf.h~a fs/xfs/xfs_buf.h
> --- 25/fs/xfs/xfs_buf.h~a	2003-08-02 00:53:59.000000000 -0700
> +++ 25-akpm/fs/xfs/xfs_buf.h	2003-08-02 00:56:03.000000000 -0700
> @@ -220,8 +220,10 @@ static inline int	xfs_bawrite(void *mp, 
>  	bp->pb_fspriv3 = mp;
>  	bp->pb_strat = xfs_bdstrat_cb;
>  	xfs_buf_undelay(bp);
> +	atomic_inc(&bp->pb_hold);
>  	if ((ret = pagebuf_iostart(bp, PBF_WRITE | PBF_ASYNC)) == 0)
>  		pagebuf_run_queues(bp);
> +	pagebuf_rele(bp);
>  	return ret;
>  }
>  
> 
> _
-- 

Steve Lord                                      voice: +1-651-683-3511
Principal Engineer, Filesystem Software         email: lord@sgi.com