lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251024102143.236665-1-mssola@mssola.com>
Date: Fri, 24 Oct 2025 12:21:39 +0200
From: Miquel Sabaté Solà <mssola@...ola.com>
To: linux-btrfs@...r.kernel.org
Cc: clm@...com,
	dsterba@...e.com,
	johannes.thumshirn@....com,
	fdmanana@...e.com,
	boris@....io,
	wqu@...e.com,
	linux-kernel@...r.kernel.org,
	Miquel Sabaté Solà <mssola@...ola.com>
Subject: [PATCH v2 0/4] btrfs: define and apply the AUTO_K(V)FREE_PTR macros

Changes since v1:
  - Remove the _PTR suffix
  - Rename the ipath cleanup function to inode_fs_paths, so it's more
    explicit on the type.
  - Improve git message in patch 1.

This patchset introduces and applies throughout the btrfs tree two new
macros: AUTO_KFREE and AUTO_KVFREE. Each macro defines a pointer,
initializes it to NULL, and sets the kfree/kvfree cleanup attribute. It was
suggested by David Sterba in the review of a patch that I submitted here
[1].

I have not applied these macros blindly through the tree, but only when
using a cleanup attribute actually made things easier for
maintainers/developers, and didn't obfuscate things like lifetimes of
objects on a given function. So, I've mostly avoided applying this when:

- The object was being re-allocated in the middle of the function
  (e.g. object re-allocation in a loop).
- The ownership of the object was transferred between functions.
- The value of a given object might depend on functions returning ERR_PTR()
  et al.
- The cleanup section of a function was a bunch of labels with different
  exit paths with non-trivial cleanup code (or code that depended on things
  to go on a specific order).

To come up with this patchset I have glanced through the tree in order to
find where and how kfree()/kvfree() were being used, and while doing so I
have submitted [2], [3] and [4] separately as they were fixing memory
related issues. All in all, this patchset can be divided in three parts:

1. Patch 1: transforms free_ipath() to be defined via DEFINE_FREE(), which
   will be useful in order to further simplify some code in patch 3.
2. Patch 2 and 3: define and use the two macros.
3. Patch 4: removing some unneeded kfree() calls from qgroup.c as they were
   not needed. Since these occurrences weren't memory bugs, and it was a
   somewhat simple patch, I've refrained from sending this separately as I
   did in [2], [3] and [4]; but I'll gladly do it if you think it's better
   for the review.

Note that after these changes some 'return' statements could be made more
explicit, and I've also written an explicit 'return 0' whenever it would
make more explicit the "happy" path for a given branch, or whenever a 'ret'
variable could be avoided that way.

Last, checkpatch.pl script doesn't seem to like patches 2 and 3; but so far
it looks like false positives to me. But of course I might just be wrong :)

[1] https://lore.kernel.org/all/20250922103442.GM5333@twin.jikos.cz/
[2] https://lore.kernel.org/all/20250925184139.403156-1-mssola@mssola.com/
[3] https://lore.kernel.org/all/20250930130452.297576-1-mssola@mssola.com/
[4] https://lore.kernel.org/all/20251008121859.440161-1-mssola@mssola.com/

Miquel Sabaté Solà (4):
  btrfs: declare free_ipath() via DEFINE_FREE()
  btrfs: define the AUTO_K(V)FREE helper macros
  btrfs: apply the AUTO_K(V)FREE macros throughout the tree
  btrfs: add ASSERTs on prealloc in qgroup functions

 fs/btrfs/acl.c                    | 29 ++++++++----------
 fs/btrfs/backref.c                | 10 +------
 fs/btrfs/backref.h                |  7 ++++-
 fs/btrfs/delayed-inode.c          | 17 +++++------
 fs/btrfs/extent-tree.c            | 17 +++++------
 fs/btrfs/inode.c                  |  4 +--
 fs/btrfs/ioctl.c                  | 34 ++++++++-------------
 fs/btrfs/misc.h                   |  7 +++++
 fs/btrfs/qgroup.c                 | 30 +++++++++++++++----
 fs/btrfs/raid-stripe-tree.c       | 14 +++------
 fs/btrfs/reflink.c                |  7 ++---
 fs/btrfs/relocation.c             | 34 ++++++++-------------
 fs/btrfs/scrub.c                  |  4 +--
 fs/btrfs/send.c                   | 50 ++++++++++++-------------------
 fs/btrfs/super.c                  |  3 +-
 fs/btrfs/tests/extent-io-tests.c  |  3 +-
 fs/btrfs/tests/extent-map-tests.c |  6 ++--
 fs/btrfs/tree-log.c               | 46 +++++++++++-----------------
 fs/btrfs/volumes.c                | 28 +++++------------
 fs/btrfs/zoned.c                  |  3 +-
 20 files changed, 145 insertions(+), 208 deletions(-)

--
2.51.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ