Re: [PATCH] xfs: fix invalid pointer dereference in xfs_attr3_node_inactive

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Feb 05, 2020 at 08:39:32AM +1100, Dave Chinner wrote:
> On Tue, Feb 04, 2020 at 03:06:36PM +0800, Zorro Lang wrote:
> > This patch fixes below KASAN report. The xfs_attr3_node_inactive()
> > gets 'child_bp' at there:
> >   error = xfs_trans_get_buf(*trans, mp->m_ddev_targp,
> >                             child_blkno,
> >                             XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0,
> >                             &child_bp);
> >   if (error)
> >           return error;
> >   error = bp->b_error;
> > 
> > But it turns to use 'bp', not 'child_bp'. And the 'bp' has been freed by:
> >   xfs_trans_brelse(*trans, bp);
> 
> ....
> > ---
> >  fs/xfs/xfs_attr_inactive.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> > index bbfa6ba84dcd..26230d150bf2 100644
> > --- a/fs/xfs/xfs_attr_inactive.c
> > +++ b/fs/xfs/xfs_attr_inactive.c
> > @@ -211,7 +211,7 @@ xfs_attr3_node_inactive(
> >  				&child_bp);
> >  		if (error)
> >  			return error;
> > -		error = bp->b_error;
> > +		error = child_bp->b_error;
> >  		if (error) {
> >  			xfs_trans_brelse(*trans, child_bp);
> >  			return error;
> 
> Isn't this dead code now? i.e. any error that occurs on the buffer
> during a xfs_trans_get_buf() call is returned directly and so it's
> caught by the "if (error)" check. Hence this whole child_bp->b_error
> check can be removed, right?

It will be after I send in the second half of the 5.6 merge window.  I
decided to hang onto the buffer error code rework until all of the
kernel fuzz tests finished running and I was satisfied with my own
userspace port of the same series.

(All that is now done, so I'll send that to linus tomorrow.)

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com