• Dave Chinner's avatar
    xfs: don't use current->journal_info · f2e812c1
    Dave Chinner authored
    syzbot reported an ext4 panic during a page fault where found a
    journal handle when it didn't expect to find one. The structure
    it tripped over had a value of 'TRAN' in the first entry in the
    structure, and that indicates it tripped over a struct xfs_trans
    instead of a jbd2 handle.
    
    The reason for this is that the page fault was taken during a
    copy-out to a user buffer from an xfs bulkstat operation. XFS uses
    an "empty" transaction context for bulkstat to do automated metadata
    buffer cleanup, and so the transaction context is valid across the
    copyout of the bulkstat info into the user buffer.
    
    We are using empty transaction contexts like this in XFS to reduce
    the risk of failing to release objects we reference during the
    operation, especially during error handling. Hence we really need to
    ensure that we can take page faults from these contexts without
    leaving landmines for the code processing the page fault to trip
    over.
    
    However, this same behaviour could happen from any other filesystem
    that triggers a page fault or any other exception that is handled
    on-stack from within a task context that has current->journal_info
    set.  Having a page fault from some other filesystem bounce into XFS
    where we have to run a transaction isn't a bug at all, but the usage
    of current->journal_info means that this could result corruption of
    the outer task's journal_info structure.
    
    The problem is purely that we now have two different contexts that
    now think they own current->journal_info. IOWs, no filesystem can
    allow page faults or on-stack exceptions while current->journal_info
    is set by the filesystem because the exception processing might use
    current->journal_info itself.
    
    If we end up with nested XFS transactions whilst holding an empty
    transaction, then it isn't an issue as the outer transaction does
    not hold a log reservation. If we ignore the current->journal_info
    usage, then the only problem that might occur is a deadlock if the
    exception tries to take the same locks the upper context holds.
    That, however, is not a problem that setting current->journal_info
    would solve, so it's largely an irrelevant concern here.
    
    IOWs, we really only use current->journal_info for a warning check
    in xfs_vm_writepages() to ensure we aren't doing writeback from a
    transaction context. Writeback might need to do allocation, so it
    can need to run transactions itself. Hence it's a debug check to
    warn us that we've done something silly, and largely it is not all
    that useful.
    
    So let's just remove all the use of current->journal_info in XFS and
    get rid of all the potential issues from nested contexts where
    current->journal_info might get misused by another filesystem
    context.
    
    Reported-by: syzbot+cdee56dbcdf0096ef605@syzkaller.appspotmail.com
    Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
    Reviewed-by: default avatar"Darrick J. Wong" <djwong@kernel.org>
    Reviewed-by: default avatarMark Tinguely <mark.tinguely@oracle.com>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarChandan Babu R <chandanbabu@kernel.org>
    f2e812c1
xfs_trans.h 9.66 KB