Re: [PATCH 2/4] xfs_db: use an empty transaction to try to prevent livelocks in path_navigate

Andrey Albershteyn
2025-02-13T13:26:32Z

On 2025-02-06 15:03:01, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> A couple of patches from now we're going to reuse the path_walk code in
> a new xfs_db subcommand that tries to recover directory trees from
> old/damaged filesystems. Let's pass around an empty transaction to try
> too avoid livelocks on malicious/broken metadata. This is not
> completely foolproof, but it's quick enough for most purposes.
>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> db/namei.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
>
> diff --git a/db/namei.c b/db/namei.c
> index 00610a54af527e..22eae50f219fd0 100644
> --- a/db/namei.c
> +++ b/db/namei.c
> @@ -87,15 +87,20 @@ path_navigate(
> xfs_ino_t rootino,
> struct dirpath *dirpath)
> {
> + struct xfs_trans *tp;
> struct xfs_inode *dp;
> xfs_ino_t ino = rootino;
> unsigned int i;
> int error;
>
> - error = -libxfs_iget(mp, NULL, ino, 0, &dp);
> + error = -libxfs_trans_alloc_empty(mp, &tp);
> if (error)
> return error;
>
> + error = -libxfs_iget(mp, tp, ino, 0, &dp);
> + if (error)
> + goto out_trans;
> +
> for (i = 0; i < dirpath->depth; i++) {
> struct xfs_name xname = {
> .name = (unsigned char *)dirpath->path[i],
> @@ -104,35 +109,37 @@ path_navigate(
>
> if (!S_ISDIR(VFS_I(dp)->i_mode)) {
> error = ENOTDIR;
> - goto rele;
> + goto out_rele;
> }
>
> - error = -libxfs_dir_lookup(NULL, dp, &xname, &ino, NULL);
> + error = -libxfs_dir_lookup(tp, dp, &xname, &ino, NULL);
> if (error)
> - goto rele;
> + goto out_rele;
> if (!xfs_verify_ino(mp, ino)) {
> error = EFSCORRUPTED;
> - goto rele;
> + goto out_rele;
> }
>
> libxfs_irele(dp);
> dp = NULL;
>
> - error = -libxfs_iget(mp, NULL, ino, 0, &dp);
> + error = -libxfs_iget(mp, tp, ino, 0, &dp);
> switch (error) {
> case EFSCORRUPTED:
> case EFSBADCRC:
> case 0:
> break;
> default:
> - return error;
> + goto out_trans;
> }
> }
>
> set_cur_inode(ino);
> -rele:
> +out_rele:
> if (dp)
> libxfs_irele(dp);
> +out_trans:
> + libxfs_trans_cancel(tp);
> return error;
> }
>
>

--
- Andrey