On Thu, Apr 19, 2018 at 11:22:19PM +0200, Luis R. Rodriguez wrote:
> On Tue, Apr 03, 2018 at 02:03:58PM +1000, Dave Chinner wrote:
> > On Mon, Apr 02, 2018 at 08:06:47PM -0700, Darrick J. Wong wrote:
> > > On Tue, Apr 03, 2018 at 11:44:58AM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > > >
> > > > When an intent is aborted during it's initial commit through
> > > > xfs_defer_trans_abort(), there is a use after free. The current
> > > > report is for a RUI through this path in generic/388:
> > > >
> > > > Freed by task 6274:
> > > > __kasan_slab_free+0x136/0x180
> > > > kmem_cache_free+0xe7/0x4b0
> > > > xfs_trans_free_items+0x198/0x2e0
> > > > __xfs_trans_commit+0x27f/0xcc0
> > > > xfs_trans_roll+0x17b/0x2a0
> > > > xfs_defer_trans_roll+0x6ad/0xe60
> > > > xfs_defer_finish+0x2a6/0x2140
> > > > xfs_alloc_file_space+0x53a/0xf90
> > > > xfs_file_fallocate+0x5c6/0xac0
> > > > vfs_fallocate+0x2f5/0x930
> > > > ioctl_preallocate+0x1dc/0x320
> > > > do_vfs_ioctl+0xfe4/0x1690
> > > >
> > > > The problem is that the RUI has two active references - one in the
> > > > current transaction, and another held by the defer_ops structure
> > > > that is passed to the RUD (intent done) so that both the intent and
> > > > the intent done structures are freed on commit of the intent done.
> > > >
> > > > Hence during abort, we need to release the intent item, because the
> > > > defer_ops reference is released separately via ->abort_intent
> > > > callback. Fix all the intent code to do this correctly.
> > > >
> > > > Signed-Off-By: Dave Chinner <dchinner@xxxxxxxxxx>
> > .....
> > > > @@ -142,7 +161,7 @@ xfs_bui_item_unlock(
> > > > struct xfs_log_item *lip)
> > > > {
> > > > if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> > >
> > > Hm, in my tree this is still "if (lip->li_flags & XFS_LI_ABORTED)" :)
> > > /me wonders where the test_bit() came from?
> >
> > Oh, sorry, I just dumped the patch on the end of my stack, which has
> > this conversion in it (because we do a bunch of racy updates on the
> > log item flags with any serialisation and that needs to be fixed to
> > get rid of log item descriptors).
> >
> > Change them all to
> >
> > if (lip->li_flags & XFS_LI_ABORTED)
> >
> > and it'll apply to the for-next tree.
>
> Just a friendly poke for resubmission, so it doesn't fall through the cracks.
0612d1166330 xfs: fix intent use-after-free on abort
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
