On Thu, Dec 28, 2023 at 06:18:29AM +0000, Christoph Hellwig wrote:
> dfp will be freed by ->recover_work and thus the tracepoint in case
> of an error can lead to a use after free.
>
> Store the defer ops in a local variable to avoid that.
>
> Fixes: 7f2f7531e0d4 ("xfs: store an ops pointer in struct xfs_defer_pending")
> Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_defer.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index ca7f0ac0489604..785c92d2acaa73 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -915,12 +915,13 @@ xfs_defer_finish_recovery(
> struct xfs_defer_pending *dfp,
> struct list_head *capture_list)
> {
> + const struct xfs_defer_op_type *ops = dfp->dfp_ops;
> int error;
>
> - error = dfp->dfp_ops->recover_work(dfp, capture_list);
> + error = ops->recover_work(dfp, capture_list);
Since I suck at remembering that dfp can be freed by recovery work, can
you add a comment to that effect? e.g.
/* dfp is freed by recover_work and must not be accessed further */
error = ops->recover_work(dfp, capture_list);
With that added,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> if (error)
> trace_xlog_intent_recovery_failed(mp, error,
> - dfp->dfp_ops->recover_work);
> + ops->recover_work);
> return error;
> }
>
> --
> 2.39.2
>
>
