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?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
