On Thu, Apr 19, 2007 at 05:53:38PM +1000, David Chinner wrote:
>
> When unmounting the filesystem we write an unmount record into the
> log just before we start freeing up in memory structures.
>
> When we wait for the unmount record to hit the disk, we don't
> wait for the log buffers to be finished with, we only wait for part of
> the iodone callback to be run - the bit that processes the
> unmount record completion.
>
> Hence when the unmount wakes up, it races with the remainder of the
> log io completion and pretty much the first thing it does is free
> the log buffers.
>
> As a result, when iodone processing completes and we check the
> buffer's async status, the buffer can already have been freed.
>
> Luckily, all log I/O is issued asynchronously, so we don't really
> need the async check and so we can avoid this use after free
> easily.
Looks good to me.
> Index: 2.6.x-xfs-new/fs/xfs/xfs_log.c
> ===================================================================
> --- 2.6.x-xfs-new.orig/fs/xfs/xfs_log.c 2007-04-19 17:18:14.097380099
> +1000
> +++ 2.6.x-xfs-new/fs/xfs/xfs_log.c 2007-04-19 17:51:03.017078512 +1000
> @@ -988,14 +988,16 @@ xlog_iodone(xfs_buf_t *bp)
> } else if (iclog->ic_state & XLOG_STATE_IOERROR) {
> aborted = XFS_LI_ABORTED;
> }
> +
> + /* log I/O is always issued ASYNC */
> + ASSERT(XFS_BUF_ISASYNC(bp));
> xlog_state_done_syncing(iclog, aborted);
> - if (!(XFS_BUF_ISASYNC(bp))) {
> - /*
> - * Corresponding psema() will be done in bwrite(). If we don't
> - * vsema() here, panic.
> - */
> - XFS_BUF_V_IODONESEMA(bp);
> - }
> + /*
> + * do not reference the buffer (bp) here as we could race
> + * with it being freed after writing the unmount record to the
> + * log.
> + */
> +
> } /* xlog_iodone */
>
> /*
>
>
---end quoted text---
|