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: <20251021142749.642956-1-mssola@mssola.com>
Date: Tue, 21 Oct 2025 16:27:45 +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,
	neal@...pa.dev,
	linux-kernel@...r.kernel.org,
	Miquel Sabaté Solà <mssola@...ola.com>
Subject: [PATCH 0/4] btrfs: define and apply the AUTO_K(V)FREE_PTR macros

This patchset introduces and applies throughout the btrfs tree two new
macros: AUTO_KFREE_PTR and AUTO_KVFREE_PTR. 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 added the _PTR suffix because I think that it reads more easily
and it makes things more explicit, but I don't have any strong feelings
about the naming either.

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() instead
  btrfs: define the AUTO_K(V)FREE_PTR helper macros
  btrfs: apply the AUTO_K(V)FREE_PTR 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