On Tue, Feb 04, 2020 at 04:09:10PM -0800, Darrick J. Wong wrote:
> 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.)
Oh, that's great! Please ignore this noise(/patch) :)
Thanks,
Zorro
>
> --D
>
> > Cheers,
> >
> > Dave.
> > --
> > Dave Chinner
> > david@xxxxxxxxxxxxx
>
