Re: [PATCH 1/4] xfs: fix buffer state when we reject a corrupt dir free block

Eric Sandeen
2020-03-02T23:54:10Z

On 2/28/20 5:48 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Fix two problems in the dir3 free block read routine when we want to
> reject a corrupt free block. First, buffers should never have DONE set
> at the same time that b_error is EFSCORRUPTED. Second, don't leak a
> pointer back to the caller.

For both of these things I'm left wondering; why does this particular
location need to have XBF_DONE cleared after the verifier error? Most
other locations that mark errors don't do this.

xfs_inode_buf_verify does, but for readahead purposes:

* If the readahead buffer is invalid, we need to mark it with an error and
* clear the DONE status of the buffer so that a followup read will re-read it
* from disk.

Also, what problem does setting the pointer to NULL solve?

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_dir2_node.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index a0cc5e240306..f622ede7119e 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -227,7 +227,9 @@ __xfs_dir3_free_read(
> fa = xfs_dir3_free_header_check(dp, fbno, *bpp);
> if (fa) {
> xfs_verifier_error(*bpp, -EFSCORRUPTED, fa);
> + (*bpp)->b_flags &= ~XBF_DONE;
> xfs_trans_brelse(tp, *bpp);
> + *bpp = NULL;
> return -EFSCORRUPTED;
> }
>
>