Commit 8d6c1210 authored by Eric Sandeen's avatar Eric Sandeen Committed by Dave Chinner
Browse files

xfs: fix buffer use after free on IO error



When testing exhaustion of dm snapshots, the following appeared
with CONFIG_DEBUG_OBJECTS_FREE enabled:

ODEBUG: free active (active state 0) object type: work_struct hint: xfs_buf_iodone_work+0x0/0x1d0 [xfs]

indicating that we'd freed a buffer which still had a pending reference,
down this path:

[  190.867975]  [<ffffffff8133e6fb>] debug_check_no_obj_freed+0x22b/0x270
[  190.880820]  [<ffffffff811da1d0>] kmem_cache_free+0xd0/0x370
[  190.892615]  [<ffffffffa02c5924>] xfs_buf_free+0xe4/0x210 [xfs]
[  190.905629]  [<ffffffffa02c6167>] xfs_buf_rele+0xe7/0x270 [xfs]
[  190.911770]  [<ffffffffa034c826>] xfs_trans_read_buf_map+0x7b6/0xac0 [xfs]

At issue is the fact that if IO fails in xfs_buf_iorequest,
we'll queue completion unconditionally, and then call
xfs_buf_rele; but if IO failed, there are no IOs remaining,
and xfs_buf_rele will free the bp while work is still queued.

Fix this by not scheduling completion if the buffer has
an error on it; run it immediately.  The rest is only comment
changes.

Thanks to dchinner for spotting the root cause.
Signed-off-by: default avatarEric Sandeen <sandeen@redhat.com>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 07d5035a
...@@ -1372,21 +1372,29 @@ xfs_buf_iorequest( ...@@ -1372,21 +1372,29 @@ xfs_buf_iorequest(
xfs_buf_wait_unpin(bp); xfs_buf_wait_unpin(bp);
xfs_buf_hold(bp); xfs_buf_hold(bp);
/* Set the count to 1 initially, this will stop an I/O /*
* Set the count to 1 initially, this will stop an I/O
* completion callout which happens before we have started * completion callout which happens before we have started
* all the I/O from calling xfs_buf_ioend too early. * all the I/O from calling xfs_buf_ioend too early.
*/ */
atomic_set(&bp->b_io_remaining, 1); atomic_set(&bp->b_io_remaining, 1);
_xfs_buf_ioapply(bp); _xfs_buf_ioapply(bp);
_xfs_buf_ioend(bp, 1); /*
* If _xfs_buf_ioapply failed, we'll get back here with
* only the reference we took above. _xfs_buf_ioend will
* drop it to zero, so we'd better not queue it for later,
* or we'll free it before it's done.
*/
_xfs_buf_ioend(bp, bp->b_error ? 0 : 1);
xfs_buf_rele(bp); xfs_buf_rele(bp);
} }
/* /*
* Waits for I/O to complete on the buffer supplied. It returns immediately if * Waits for I/O to complete on the buffer supplied. It returns immediately if
* no I/O is pending or there is already a pending error on the buffer. It * no I/O is pending or there is already a pending error on the buffer, in which
* returns the I/O error code, if any, or 0 if there was no error. * case nothing will ever complete. It returns the I/O error code, if any, or
* 0 if there was no error.
*/ */
int int
xfs_buf_iowait( xfs_buf_iowait(
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment