Re: [PATCH 7/9] xfs: separate out log shutdown callback processing

[Dave Chinner <david@fromorbit.com>]

On Fri, Jul 02, 2021 at 09:36:28AM +0100, Christoph Hellwig wrote:
> On Wed, Jun 30, 2021 at 04:38:11PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > The iclog callback processing done during a forced log shutdown has
> > different logic to normal runtime IO completion callback processing.
> > Separate out eh shutdown callbacks into their own function and call
> > that from the shutdown code instead.
> > 
> > We don't need this shutdown specific logic in the normal runtime
> > completion code - we'll always run the shutdown version on shutdown,
> > and it will do what shutdown needs regardless of whether there are
> > racing IO completion callbacks scheduled or in progress. Hence we
> > can also simplify the normal IO completion callpath and only abort
> > if shutdown occurred while we actively were processing callbacks.
> 
> What prevents a log shutdown from coming in during the callback
> processing?  Or is there a reason why we simply don't care for that
> case?

We simpy don't care. IO completion based callbacks can already race
with shutdown driven callbacks. RIght now, both cases will process
all iclogs, so it just depends on which one gets to the iclog first
as to which one runs the callbacks. We don't actually pass the
shutdown state to the callbacks, so the callbacks are none-the-wiser
for whether they are being called from shutdown or IO completion
when they see the shutdown state.

With the code as per this patch, a racing shutdown will result in
the callbacks from IO completion seeing the shutdown state, but IO
completion will now avoid processing iclogs out of order/statei
because the shutdown state is set. Hence by taking out the shutdown
check from IO completion, we avoid having IO completion based
callbacks racing with referenced iclogs that have just attached
callbacks to the iclog but haven't yet released their reference and
submitted the iclog for IO...

IOWs, it's not just the callbacks running from shutdown that trigger
the UAF problems with callbacks, it can also occur from IO
completion, too. Hence we really need to separate out the
shutdown case from the IO completion path to avoid it from having
the same problem(s) as the shutdown path...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com