| 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
| ||
|
Message-ID: <20251022073138.GX13776@twin.jikos.cz>
Date: Wed, 22 Oct 2025 09:31:38 +0200
From: David Sterba <dsterba@...e.cz>
To: Miquel Sabaté Solà <mssola@...ola.com>
Cc: linux-btrfs@...r.kernel.org, clm@...com, dsterba@...e.com,
johannes.thumshirn@....com, fdmanana@...e.com, boris@....io,
wqu@...e.com, neal@...pa.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] btrfs: declare free_ipath() via DEFINE_FREE() instead
On Tue, Oct 21, 2025 at 04:27:46PM +0200, Miquel Sabaté Solà wrote:
> This transforms the signature to __free_ipath() instead of the original
> free_ipath(), but this function was only being used as a cleanup
> function anyways. Hence, define it as a helper and use it via the
> __free() attribute on all uses. This change also means that
> __free_ipath() will be inlined whereas that wasn't the case for the
> original one, but this shouldn't be a problem.
>
> A follow up macro like we do with BTRFS_PATH_AUTO_FREE() has been
> discarded as the usage of this struct is not as widespread as that.
The point of adding the macros or at least the freeing wrappers is to
force the NULL initialization and to make it visible in the declarations
(typed all in capitals). The number of use should not be the main factor
and in this case it's 4 files.
> Signed-off-by: Miquel Sabaté Solà <mssola@...ola.com>
> ---
> fs/btrfs/backref.c | 10 +---------
> fs/btrfs/backref.h | 7 ++++++-
> fs/btrfs/inode.c | 4 +---
> fs/btrfs/ioctl.c | 3 +--
> fs/btrfs/scrub.c | 4 +---
> 5 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index e050d0938dc4..a1456402752a 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2785,7 +2785,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes)
> * allocates space to return multiple file system paths for an inode.
> * total_bytes to allocate are passed, note that space usable for actual path
> * information will be total_bytes - sizeof(struct inode_fs_paths).
> - * the returned pointer must be freed with free_ipath() in the end.
> + * the returned pointer must be freed with __free_ipath() in the end.
> */
> struct inode_fs_paths *init_ipath(s32 total_bytes, struct btrfs_root *fs_root,
> struct btrfs_path *path)
> @@ -2810,14 +2810,6 @@ struct inode_fs_paths *init_ipath(s32 total_bytes, struct btrfs_root *fs_root,
> return ifp;
> }
>
> -void free_ipath(struct inode_fs_paths *ipath)
> -{
> - if (!ipath)
> - return;
> - kvfree(ipath->fspath);
> - kfree(ipath);
> -}
> -
> struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> {
> struct btrfs_backref_iter *ret;
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 25d51c246070..d3b1ad281793 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -241,7 +241,12 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root, struct btrfs_path *path,
> struct btrfs_data_container *init_data_container(u32 total_bytes);
> struct inode_fs_paths *init_ipath(s32 total_bytes, struct btrfs_root *fs_root,
> struct btrfs_path *path);
> -void free_ipath(struct inode_fs_paths *ipath);
> +
> +DEFINE_FREE(ipath, struct inode_fs_paths *,
> + if (_T) {
You can drop the if() as kvfree/kfree handles NULL pointers and we don't
do that in the struct btrfs_path either.
> + kvfree(_T->fspath);
> + kfree(_T);
> + })
>
> int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
> u64 start_off, struct btrfs_path *path,
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 79732756b87f..4d154209d70b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -130,7 +130,7 @@ static int data_reloc_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> struct btrfs_fs_info *fs_info = warn->fs_info;
> struct extent_buffer *eb;
> struct btrfs_inode_item *inode_item;
> - struct inode_fs_paths *ipath = NULL;
> + struct inode_fs_paths *ipath __free(ipath) = NULL;
I'd spell the name in full, like __free(free_ipath) or
__free(inode_fs_paths) so it matches the type not the variable name.
> struct btrfs_root *local_root;
> struct btrfs_key key;
> unsigned int nofs_flag;
> @@ -193,7 +193,6 @@ static int data_reloc_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> }
>
> btrfs_put_root(local_root);
> - free_ipath(ipath);
> return 0;
>
> err:
> @@ -201,7 +200,6 @@ static int data_reloc_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> "checksum error at logical %llu mirror %u root %llu inode %llu offset %llu, path resolving failed with ret=%d",
> warn->logical, warn->mirror_num, root, inum, offset, ret);
>
> - free_ipath(ipath);
> return ret;
> }
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 692016b2b600..453832ded917 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3298,7 +3298,7 @@ static long btrfs_ioctl_ino_to_path(struct btrfs_root *root, void __user *arg)
> u64 rel_ptr;
> int size;
> struct btrfs_ioctl_ino_path_args *ipa = NULL;
> - struct inode_fs_paths *ipath = NULL;
> + struct inode_fs_paths *ipath __free(ipath) = NULL;
> struct btrfs_path *path;
>
> if (!capable(CAP_DAC_READ_SEARCH))
> @@ -3346,7 +3346,6 @@ static long btrfs_ioctl_ino_to_path(struct btrfs_root *root, void __user *arg)
>
> out:
> btrfs_free_path(path);
> - free_ipath(ipath);
> kfree(ipa);
>
> return ret;
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index fe266785804e..74d8af1ff02d 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -505,7 +505,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> struct btrfs_inode_item *inode_item;
> struct scrub_warning *swarn = warn_ctx;
> struct btrfs_fs_info *fs_info = swarn->dev->fs_info;
> - struct inode_fs_paths *ipath = NULL;
> + struct inode_fs_paths *ipath __free(ipath) = NULL;
> struct btrfs_root *local_root;
> struct btrfs_key key;
>
> @@ -569,7 +569,6 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> (char *)(unsigned long)ipath->fspath->val[i]);
>
> btrfs_put_root(local_root);
> - free_ipath(ipath);
> return 0;
>
> err:
> @@ -580,7 +579,6 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 num_bytes,
> swarn->physical,
> root, inum, offset, ret);
>
> - free_ipath(ipath);
> return 0;
> }
>
> --
> 2.51.1
>
Powered by blists - more mailing lists