Re: [PATCH 00/12] btrfs-progs: fix btrfs_path leak, part 2

[Qu Wenruo <wqu@suse.com>]

在 2026/10/30 14:45, Qu Wenruo 写道:
> This is the part 2, focusing on the on-stack btrfs_path.

This obviously rely on the part 1.

For the full patchset series, one can fetch it from the github PR:

https://github.com/kdave/btrfs-progs/pull/1191

> 
> Unlike heap-allocated btrfs_path, which needs proper btrfs_free_path()
> call, or the btrfs_path itself along with the held ebs will leak.
> 
> The on-stack one only leaks the held ebs, but still we should fix them.
> 
> Just by the amount of code, one should know this is largely generated by
> LLM, although I spent more time drafting the plan and review the final
> patch.
> 
> There are some extra notes other than just converting existing
> "btrfs_path" on-stack declaration to BTRFS_PATH_AUTO_RELEASE:
> 
> - If a label is only for cleanup, remove the label when possible
>    As a lot of call sites can directly return without jumping to the
>    cleanup label.
> 
>    Also cleanup some "ret = x; return ret;" patterns.
> 
> - Keep dedicated btrfs_release_path() if needed
>    As our C code is not fully aligning the path lifespan with the scope,
>    we still need to manually release path halfway for certain call sites.
> 
>    E.g. before calling btrfs_commit_transaction(),
>    btrfs_abort_transaction(), btrfs_search_slot(), close_ctree() or
>    before lowmem mode modifying the tree.
> 
>    Releasing the path before btrfs_commit_transaction() is to follow the
>    kernel convension. There are a lot of existing call sites breaking it,
>    so here we do not intentionally fix them.
> 
>    This will require a dedicated change in the future.
> 
> - Reduce the lifespan of btrfs_path when possible
>    E.g. the path is only utilized inside a loop or if () branch, declare
>    path there.
> 
> This applies to patch 01~08, which are all straightforward conversion
> with minor cleanups.
> 
> After that, the code review process exposed some other bugs in the
> touched functions, which are all pretty simple and low severity ones.
> Patch 09~12 are to fix those minor bugs.
> 
> Qu Wenruo (12):
>    btrfs-progs: kernel-shared: use BTRFS_PATH_AUTO_RELEASE() for on-stack
>      paths
>    btrfs-progs: common, convert, image: use BTRFS_PATH_AUTO_RELEASE() for
>      on-stack paths
>    btrfs-progs: mkfs: use BTRFS_PATH_AUTO_RELEASE() for on-stack paths
>    btrfs-progs: tune: use BTRFS_PATH_AUTO_RELEASE() for on-stack paths
>    btrfs-progs: cmds: use BTRFS_PATH_AUTO_RELEASE() for on-stack paths
>    btrfs-progs: check: use BTRFS_PATH_AUTO_RELEASE() for repair,
>      qgroup-verify and mode-common
>    btrfs-progs: check: use BTRFS_PATH_AUTO_RELEASE() for lowmem mode
>    btrfs-progs: check: use BTRFS_PATH_AUTO_RELEASE() for original mode
>    btrfs-progs: check: fix fixup_extent_flags() retry path
>    btrfs-progs: fix missing transaction commit in
>      truncate_free_ino_items()
>    btrfs-progs: dump-tree: fix exit code and leak on open failure
>    btrfs-progs: check: return error if fill_csum_tree_from_fs() fails to
>      read a root
> 
>   check/main.c                     | 235 +++++++++++--------------------
>   check/mode-common.c              | 141 +++++++------------
>   check/mode-lowmem.c              | 202 +++++++++-----------------
>   check/qgroup-verify.c            |  29 ++--
>   check/repair.c                   |   5 +-
>   cmds/device.c                    |  11 +-
>   cmds/inspect-dump-tree.c         |  11 +-
>   cmds/rescue-chunk-recover.c      |  43 ++----
>   cmds/rescue-fix-data-checksum.c  |  10 +-
>   cmds/rescue.c                    |   7 +-
>   cmds/restore.c                   | 126 +++++++----------
>   common/clear-cache.c             |  79 ++++-------
>   common/root-tree-utils.c         |  10 +-
>   convert/main.c                   |   9 +-
>   convert/source-ext2.c            |   3 +-
>   convert/source-fs.c              |   3 +-
>   image/common.c                   |   2 +-
>   image/image-create.c             |   2 +-
>   image/image-restore.c            |  20 +--
>   kernel-shared/extent-tree.c      |  62 +++-----
>   kernel-shared/file.c             |   3 +-
>   kernel-shared/free-space-cache.c |  24 ++--
>   kernel-shared/inode.c            |   8 +-
>   kernel-shared/print-tree.c       |   9 +-
>   kernel-shared/volumes.c          |  15 +-
>   mkfs/main.c                      |  42 ++----
>   mkfs/rootdir.c                   |  36 ++---
>   tests/btree-test.c               |   2 +-
>   tune/change-csum.c               |  50 ++-----
>   tune/change-uuid.c               |  38 ++---
>   tune/convert-remap-tree.c        |  12 +-
>   tune/quota.c                     |  19 ++-
>   32 files changed, 447 insertions(+), 821 deletions(-)
> 
> --
> 2.56.0