* [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc
@ 2025-09-19 14:58 Miquel Sabaté Solà
2025-09-19 14:58 ` [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc Miquel Sabaté Solà
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Miquel Sabaté Solà @ 2025-09-19 14:58 UTC (permalink / raw)
To: linux-btrfs; +Cc: clm, dsterba, linux-kernel, Miquel Sabaté Solà
Hello,
There were a couple of instances in btrfs code in which kmalloc was being
used with open-coded arithmetic. This can lead into unfortunate overflow
situations as describbed here [1]. The solution is to use kmalloc_array in
these cases, which is what it's being done in my first patch.
The second patch is a small cleanup after fixing up my first patch, in
which I realized that the __free(kfree) attribute would come in handy in a
couple of particularly large functions with multiple exit points. This
second patch is probably more of a cosmetic thing, and it's not an
exhaustive exercise by any means. All of this to say that even if I feel
like it should be included, I don't mind if it has to be dropped.
Cheers,
Miquel
[1] Documentation/process/deprecated.rst
Miquel Sabaté Solà (2):
btrfs: Prevent open-coded arithmetic in kmalloc
btrfs: Prefer using the __free cleanup attribute
fs/btrfs/delayed-inode.c | 18 ++++++++----------
fs/btrfs/tree-log.c | 30 +++++++++++-------------------
2 files changed, 19 insertions(+), 29 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-19 14:58 [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc Miquel Sabaté Solà @ 2025-09-19 14:58 ` Miquel Sabaté Solà 2025-09-22 10:28 ` David Sterba 2025-09-19 14:58 ` [PATCH 2/2] btrfs: Prefer using the __free cleanup attribute Miquel Sabaté Solà 2025-09-22 10:34 ` [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc David Sterba 2 siblings, 1 reply; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-19 14:58 UTC (permalink / raw) To: linux-btrfs; +Cc: clm, dsterba, linux-kernel, Miquel Sabaté Solà As pointed out in the documentation, calling 'kmalloc' with open-coded arithmetic can lead to unfortunate overflows and this particular way of using it has been deprecated. Instead, it's preferred to use 'kmalloc_array' in cases where it might apply so an overflow check is performed. Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com> --- fs/btrfs/delayed-inode.c | 4 ++-- fs/btrfs/tree-log.c | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index 6adfe62cd0c4..81577a0c601f 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -738,8 +738,8 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans, u32 *ins_sizes; int i = 0; - ins_data = kmalloc(batch.nr * sizeof(u32) + - batch.nr * sizeof(struct btrfs_key), GFP_NOFS); + ins_data = kmalloc_array(batch.nr, + sizeof(u32) + sizeof(struct btrfs_key), GFP_NOFS); if (!ins_data) { ret = -ENOMEM; goto out; diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 7d19a8c5b2a3..d6471cd33f7f 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -4062,8 +4062,7 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans, struct btrfs_key *ins_keys; u32 *ins_sizes; - ins_data = kmalloc(count * sizeof(u32) + - count * sizeof(struct btrfs_key), GFP_NOFS); + ins_data = kmalloc_array(count, sizeof(u32) + sizeof(struct btrfs_key), GFP_NOFS); if (!ins_data) return -ENOMEM; @@ -4826,8 +4825,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, src = src_path->nodes[0]; - ins_data = kmalloc(nr * sizeof(struct btrfs_key) + - nr * sizeof(u32), GFP_NOFS); + ins_data = kmalloc_array(nr, sizeof(struct btrfs_key) + sizeof(u32), GFP_NOFS); if (!ins_data) return -ENOMEM; @@ -6532,8 +6530,7 @@ static int log_delayed_insertion_items(struct btrfs_trans_handle *trans, if (!first) return 0; - ins_data = kmalloc(max_batch_size * sizeof(u32) + - max_batch_size * sizeof(struct btrfs_key), GFP_NOFS); + ins_data = kmalloc_array(max_batch_size, sizeof(u32) + sizeof(struct btrfs_key), GFP_NOFS); if (!ins_data) return -ENOMEM; ins_sizes = (u32 *)ins_data; -- 2.51.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-19 14:58 ` [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc Miquel Sabaté Solà @ 2025-09-22 10:28 ` David Sterba 2025-09-22 12:47 ` Miquel Sabaté Solà 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2025-09-22 10:28 UTC (permalink / raw) To: Miquel Sabaté Solà; +Cc: linux-btrfs, clm, dsterba, linux-kernel On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: > As pointed out in the documentation, calling 'kmalloc' with open-coded > arithmetic can lead to unfortunate overflows and this particular way of > using it has been deprecated. Instead, it's preferred to use > 'kmalloc_array' in cases where it might apply so an overflow check is > performed. So this is an API cleanup and it makes sense to use the checked multiplication but it should be also said that this is not fixing any overflow because in all cases the multipliers are bounded small numbers derived from number of items in leaves/nodes. > Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com> Reviewed-by: David Sterba <dsterba@suse.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-22 10:28 ` David Sterba @ 2025-09-22 12:47 ` Miquel Sabaté Solà 2025-09-23 6:13 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-22 12:47 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs, clm, dsterba, linux-kernel [-- Attachment #1: Type: text/plain, Size: 987 bytes --] Hello, David Sterba @ 2025-09-22 12:28 +02: > On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: >> As pointed out in the documentation, calling 'kmalloc' with open-coded >> arithmetic can lead to unfortunate overflows and this particular way of >> using it has been deprecated. Instead, it's preferred to use >> 'kmalloc_array' in cases where it might apply so an overflow check is >> performed. > > So this is an API cleanup and it makes sense to use the checked > multiplication but it should be also said that this is not fixing any > overflow because in all cases the multipliers are bounded small numbers > derived from number of items in leaves/nodes. Yes, it's just an API cleanup and I don't think it fixes any current bug in the code base. So no need to CC stable or anything like that. > >> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com> > > Reviewed-by: David Sterba <dsterba@suse.com> Thanks for the review! Miquel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 897 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-22 12:47 ` Miquel Sabaté Solà @ 2025-09-23 6:13 ` David Sterba 2025-09-23 6:47 ` Miquel Sabaté Solà 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2025-09-23 6:13 UTC (permalink / raw) To: Miquel Sabaté Solà Cc: David Sterba, linux-btrfs, clm, dsterba, linux-kernel On Mon, Sep 22, 2025 at 02:47:13PM +0200, Miquel Sabaté Solà wrote: > Hello, > > David Sterba @ 2025-09-22 12:28 +02: > > > On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: > >> As pointed out in the documentation, calling 'kmalloc' with open-coded > >> arithmetic can lead to unfortunate overflows and this particular way of > >> using it has been deprecated. Instead, it's preferred to use > >> 'kmalloc_array' in cases where it might apply so an overflow check is > >> performed. > > > > So this is an API cleanup and it makes sense to use the checked > > multiplication but it should be also said that this is not fixing any > > overflow because in all cases the multipliers are bounded small numbers > > derived from number of items in leaves/nodes. > > Yes, it's just an API cleanup and I don't think it fixes any current bug > in the code base. So no need to CC stable or anything like that. Still the changelog should say explicitly that it's not a bug fix before somebody assigns a CVE to it because it mentions overflow. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-23 6:13 ` David Sterba @ 2025-09-23 6:47 ` Miquel Sabaté Solà 2025-09-23 7:00 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-23 6:47 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs, clm, dsterba, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1220 bytes --] David Sterba @ 2025-09-23 08:13 +02: > On Mon, Sep 22, 2025 at 02:47:13PM +0200, Miquel Sabaté Solà wrote: >> Hello, >> >> David Sterba @ 2025-09-22 12:28 +02: >> >> > On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: >> >> As pointed out in the documentation, calling 'kmalloc' with open-coded >> >> arithmetic can lead to unfortunate overflows and this particular way of >> >> using it has been deprecated. Instead, it's preferred to use >> >> 'kmalloc_array' in cases where it might apply so an overflow check is >> >> performed. >> > >> > So this is an API cleanup and it makes sense to use the checked >> > multiplication but it should be also said that this is not fixing any >> > overflow because in all cases the multipliers are bounded small numbers >> > derived from number of items in leaves/nodes. >> >> Yes, it's just an API cleanup and I don't think it fixes any current bug >> in the code base. So no need to CC stable or anything like that. > > Still the changelog should say explicitly that it's not a bug fix before > somebody assigns a CVE to it because it mentions overflow. Got it! I will submit a v2 and make this more explicit. Thanks, Miquel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 897 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-23 6:47 ` Miquel Sabaté Solà @ 2025-09-23 7:00 ` David Sterba 2025-09-23 8:00 ` Miquel Sabaté Solà 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2025-09-23 7:00 UTC (permalink / raw) To: Miquel Sabaté Solà Cc: David Sterba, linux-btrfs, clm, dsterba, linux-kernel On Tue, Sep 23, 2025 at 08:47:35AM +0200, Miquel Sabaté Solà wrote: > David Sterba @ 2025-09-23 08:13 +02: > > > On Mon, Sep 22, 2025 at 02:47:13PM +0200, Miquel Sabaté Solà wrote: > >> Hello, > >> > >> David Sterba @ 2025-09-22 12:28 +02: > >> > >> > On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: > >> >> As pointed out in the documentation, calling 'kmalloc' with open-coded > >> >> arithmetic can lead to unfortunate overflows and this particular way of > >> >> using it has been deprecated. Instead, it's preferred to use > >> >> 'kmalloc_array' in cases where it might apply so an overflow check is > >> >> performed. > >> > > >> > So this is an API cleanup and it makes sense to use the checked > >> > multiplication but it should be also said that this is not fixing any > >> > overflow because in all cases the multipliers are bounded small numbers > >> > derived from number of items in leaves/nodes. > >> > >> Yes, it's just an API cleanup and I don't think it fixes any current bug > >> in the code base. So no need to CC stable or anything like that. > > > > Still the changelog should say explicitly that it's not a bug fix before > > somebody assigns a CVE to it because it mentions overflow. > > Got it! I will submit a v2 and make this more explicit. No need to, I've updated the changelog at commit time. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc 2025-09-23 7:00 ` David Sterba @ 2025-09-23 8:00 ` Miquel Sabaté Solà 0 siblings, 0 replies; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-23 8:00 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs, clm, dsterba, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1558 bytes --] David Sterba @ 2025-09-23 09:00 +02: > On Tue, Sep 23, 2025 at 08:47:35AM +0200, Miquel Sabaté Solà wrote: >> David Sterba @ 2025-09-23 08:13 +02: >> >> > On Mon, Sep 22, 2025 at 02:47:13PM +0200, Miquel Sabaté Solà wrote: >> >> Hello, >> >> >> >> David Sterba @ 2025-09-22 12:28 +02: >> >> >> >> > On Fri, Sep 19, 2025 at 04:58:15PM +0200, Miquel Sabaté Solà wrote: >> >> >> As pointed out in the documentation, calling 'kmalloc' with open-coded >> >> >> arithmetic can lead to unfortunate overflows and this particular way of >> >> >> using it has been deprecated. Instead, it's preferred to use >> >> >> 'kmalloc_array' in cases where it might apply so an overflow check is >> >> >> performed. >> >> > >> >> > So this is an API cleanup and it makes sense to use the checked >> >> > multiplication but it should be also said that this is not fixing any >> >> > overflow because in all cases the multipliers are bounded small numbers >> >> > derived from number of items in leaves/nodes. >> >> >> >> Yes, it's just an API cleanup and I don't think it fixes any current bug >> >> in the code base. So no need to CC stable or anything like that. >> > >> > Still the changelog should say explicitly that it's not a bug fix before >> > somebody assigns a CVE to it because it mentions overflow. >> >> Got it! I will submit a v2 and make this more explicit. > > No need to, I've updated the changelog at commit time. Ah, then ignore the v2 patch set I've just sent, which simply changes the commit log as you suggested. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 897 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] btrfs: Prefer using the __free cleanup attribute 2025-09-19 14:58 [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc Miquel Sabaté Solà 2025-09-19 14:58 ` [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc Miquel Sabaté Solà @ 2025-09-19 14:58 ` Miquel Sabaté Solà 2025-09-22 10:34 ` [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc David Sterba 2 siblings, 0 replies; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-19 14:58 UTC (permalink / raw) To: linux-btrfs; +Cc: clm, dsterba, linux-kernel, Miquel Sabaté Solà In delayed-node.c and tree-log.c there were a couple of instances where the __free(kfree) cleanup attribute allowed for more clear code and safer for future changes, as they were in large functions with multiple exit points and the end cleanup code was simply calling 'kfree' for the ins_data variable. Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com> --- fs/btrfs/delayed-inode.c | 14 ++++++-------- fs/btrfs/tree-log.c | 21 ++++++++------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index 81577a0c601f..dbc63dc414bb 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -668,7 +668,7 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans, struct btrfs_key first_key; const u32 first_data_size = first_item->data_len; int total_size; - char *ins_data = NULL; + char *ins_data __free(kfree) = NULL; int ret; bool continuous_keys_only = false; @@ -740,10 +740,9 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans, ins_data = kmalloc_array(batch.nr, sizeof(u32) + sizeof(struct btrfs_key), GFP_NOFS); - if (!ins_data) { - ret = -ENOMEM; - goto out; - } + if (!ins_data) + return -ENOMEM; + ins_sizes = (u32 *)ins_data; ins_keys = (struct btrfs_key *)(ins_data + batch.nr * sizeof(u32)); batch.keys = ins_keys; @@ -759,7 +758,7 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans, ret = btrfs_insert_empty_items(trans, root, path, &batch); if (ret) - goto out; + return ret; list_for_each_entry(curr, &item_list, tree_list) { char *data_ptr; @@ -814,8 +813,7 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans, list_del(&curr->tree_list); btrfs_release_delayed_item(curr); } -out: - kfree(ins_data); + return ret; } diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index d6471cd33f7f..c376514cf844 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -3302,7 +3302,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root *root, mutex_unlock(&root->log_mutex); } -/* +/* * Invoked in log mutex context, or be sure there is no other task which * can access the list. */ @@ -4038,7 +4038,7 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans, int count) { struct btrfs_root *log = inode->root->log_root; - char *ins_data = NULL; + char *ins_data __free(kfree) = NULL; struct btrfs_item_batch batch; struct extent_buffer *dst; unsigned long src_offset; @@ -4083,7 +4083,7 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans, ret = btrfs_insert_empty_items(trans, log, dst_path, &batch); if (ret) - goto out; + return ret; dst = dst_path->nodes[0]; /* @@ -4115,8 +4115,6 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans, if (btrfs_get_first_dir_index_to_log(inode) == 0) btrfs_set_first_dir_index_to_log(inode, batch.keys[0].offset); -out: - kfree(ins_data); return ret; } @@ -4786,7 +4784,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, struct btrfs_key *ins_keys; u32 *ins_sizes; struct btrfs_item_batch batch; - char *ins_data; + char *ins_data __free(kfree) = NULL; int dst_index; const bool skip_csum = (inode->flags & BTRFS_INODE_NODATASUM); const u64 i_size = i_size_read(&inode->vfs_inode); @@ -4914,7 +4912,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, disk_bytenr + extent_num_bytes - 1, &ordered_sums, false); if (ret < 0) - goto out; + return ret; ret = 0; list_for_each_entry_safe(sums, sums_next, &ordered_sums, list) { @@ -4924,7 +4922,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, kfree(sums); } if (ret) - goto out; + return ret; add_to_batch: ins_sizes[dst_index] = btrfs_item_size(src, src_slot); @@ -4938,11 +4936,11 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, * so we don't need to do anything. */ if (batch.nr == 0) - goto out; + return 0; ret = btrfs_insert_empty_items(trans, log, dst_path, &batch); if (ret) - goto out; + return ret; dst_index = 0; for (int i = 0; i < nr; i++) { @@ -4995,8 +4993,6 @@ static noinline int copy_items(struct btrfs_trans_handle *trans, } btrfs_release_path(dst_path); -out: - kfree(ins_data); return ret; } @@ -8082,4 +8078,3 @@ void btrfs_log_new_name(struct btrfs_trans_handle *trans, btrfs_end_log_trans(root); free_extent_buffer(ctx.scratch_eb); } - -- 2.51.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc 2025-09-19 14:58 [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc Miquel Sabaté Solà 2025-09-19 14:58 ` [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc Miquel Sabaté Solà 2025-09-19 14:58 ` [PATCH 2/2] btrfs: Prefer using the __free cleanup attribute Miquel Sabaté Solà @ 2025-09-22 10:34 ` David Sterba 2025-09-22 12:51 ` Miquel Sabaté Solà 2 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2025-09-22 10:34 UTC (permalink / raw) To: Miquel Sabaté Solà; +Cc: linux-btrfs, clm, dsterba, linux-kernel On Fri, Sep 19, 2025 at 04:58:14PM +0200, Miquel Sabaté Solà wrote: > The second patch is a small cleanup after fixing up my first patch, in > which I realized that the __free(kfree) attribute would come in handy in a > couple of particularly large functions with multiple exit points. This > second patch is probably more of a cosmetic thing, and it's not an > exhaustive exercise by any means. All of this to say that even if I feel > like it should be included, I don't mind if it has to be dropped. Yes there are many candidates for the __free() cleanup annotation and we'll want to fix them all systematically. We already have the automatic cleaning for struct btrfs_path (BTRFS_PATH_AUTO_FREE). For the kfree/kvfree I'd like to something similar: #define AUTO_KFREE(name) *name __free(kfree) = NULL #define AUTO_KVFREE(name) *name __free(kvfree) = NULL This wraps the name and initializes it to NULL so it's not accidentally forgotten. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc 2025-09-22 10:34 ` [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc David Sterba @ 2025-09-22 12:51 ` Miquel Sabaté Solà 2025-09-23 6:11 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-22 12:51 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs, clm, dsterba, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1420 bytes --] Hello, David Sterba @ 2025-09-22 12:34 +02: > On Fri, Sep 19, 2025 at 04:58:14PM +0200, Miquel Sabaté Solà wrote: >> The second patch is a small cleanup after fixing up my first patch, in >> which I realized that the __free(kfree) attribute would come in handy in a >> couple of particularly large functions with multiple exit points. This >> second patch is probably more of a cosmetic thing, and it's not an >> exhaustive exercise by any means. All of this to say that even if I feel >> like it should be included, I don't mind if it has to be dropped. > > Yes there are many candidates for the __free() cleanup annotation and > we'll want to fix them all systematically. We already have the automatic > cleaning for struct btrfs_path (BTRFS_PATH_AUTO_FREE). For the > kfree/kvfree I'd like to something similar: > > #define AUTO_KFREE(name) *name __free(kfree) = NULL > #define AUTO_KVFREE(name) *name __free(kvfree) = NULL > > This wraps the name and initializes it to NULL so it's not accidentally > forgotten. Makes sense! I can take a look at this if nobody else is working on it, even if I think it should go into a new patch series. Hence, if it sounds good to you, we can merge this patch as it is right now, and in parallel I work on this proposed AUTO_KFREE and AUTO_KVFREE macros in a new patch series (which will take more time to prepare). Thanks, Miquel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 897 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc 2025-09-22 12:51 ` Miquel Sabaté Solà @ 2025-09-23 6:11 ` David Sterba 2025-09-23 6:46 ` Miquel Sabaté Solà 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2025-09-23 6:11 UTC (permalink / raw) To: Miquel Sabaté Solà Cc: David Sterba, linux-btrfs, clm, dsterba, linux-kernel On Mon, Sep 22, 2025 at 02:51:15PM +0200, Miquel Sabaté Solà wrote: > > On Fri, Sep 19, 2025 at 04:58:14PM +0200, Miquel Sabaté Solà wrote: > >> The second patch is a small cleanup after fixing up my first patch, in > >> which I realized that the __free(kfree) attribute would come in handy in a > >> couple of particularly large functions with multiple exit points. This > >> second patch is probably more of a cosmetic thing, and it's not an > >> exhaustive exercise by any means. All of this to say that even if I feel > >> like it should be included, I don't mind if it has to be dropped. > > > > Yes there are many candidates for the __free() cleanup annotation and > > we'll want to fix them all systematically. We already have the automatic > > cleaning for struct btrfs_path (BTRFS_PATH_AUTO_FREE). For the > > kfree/kvfree I'd like to something similar: > > > > #define AUTO_KFREE(name) *name __free(kfree) = NULL > > #define AUTO_KVFREE(name) *name __free(kvfree) = NULL > > > > This wraps the name and initializes it to NULL so it's not accidentally > > forgotten. > > Makes sense! I can take a look at this if nobody else is working on it, > even if I think it should go into a new patch series. Thanks, it's yours. Yes this should be in a separate patchset. > Hence, if it sounds good to you, we can merge this patch as it is right > now, and in parallel I work on this proposed AUTO_KFREE and AUTO_KVFREE > macros in a new patch series (which will take more time to prepare). I'd rather see all the changes done the same way so it's not __free and then converted to AUTO_KFREE. Also the development branch is frozen before 6.18 pull request so all that will be in the 6.19 cycle anyway. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc 2025-09-23 6:11 ` David Sterba @ 2025-09-23 6:46 ` Miquel Sabaté Solà 0 siblings, 0 replies; 13+ messages in thread From: Miquel Sabaté Solà @ 2025-09-23 6:46 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs, clm, dsterba, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1942 bytes --] David Sterba @ 2025-09-23 08:11 +02: > On Mon, Sep 22, 2025 at 02:51:15PM +0200, Miquel Sabaté Solà wrote: >> > On Fri, Sep 19, 2025 at 04:58:14PM +0200, Miquel Sabaté Solà wrote: >> >> The second patch is a small cleanup after fixing up my first patch, in >> >> which I realized that the __free(kfree) attribute would come in handy in a >> >> couple of particularly large functions with multiple exit points. This >> >> second patch is probably more of a cosmetic thing, and it's not an >> >> exhaustive exercise by any means. All of this to say that even if I feel >> >> like it should be included, I don't mind if it has to be dropped. >> > >> > Yes there are many candidates for the __free() cleanup annotation and >> > we'll want to fix them all systematically. We already have the automatic >> > cleaning for struct btrfs_path (BTRFS_PATH_AUTO_FREE). For the >> > kfree/kvfree I'd like to something similar: >> > >> > #define AUTO_KFREE(name) *name __free(kfree) = NULL >> > #define AUTO_KVFREE(name) *name __free(kvfree) = NULL >> > >> > This wraps the name and initializes it to NULL so it's not accidentally >> > forgotten. >> >> Makes sense! I can take a look at this if nobody else is working on it, >> even if I think it should go into a new patch series. > > Thanks, it's yours. Yes this should be in a separate patchset. Great, will do! > >> Hence, if it sounds good to you, we can merge this patch as it is right >> now, and in parallel I work on this proposed AUTO_KFREE and AUTO_KVFREE >> macros in a new patch series (which will take more time to prepare). > > I'd rather see all the changes done the same way so it's not __free and > then converted to AUTO_KFREE. Also the development branch is frozen > before 6.18 pull request so all that will be in the 6.19 cycle anyway. Got it. Then in v2 I will drop this in favor of the later patchset. Greetings, Miquel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 897 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-09-23 8:00 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-19 14:58 [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc Miquel Sabaté Solà 2025-09-19 14:58 ` [PATCH 1/2] btrfs: Prevent open-coded arithmetic in kmalloc Miquel Sabaté Solà 2025-09-22 10:28 ` David Sterba 2025-09-22 12:47 ` Miquel Sabaté Solà 2025-09-23 6:13 ` David Sterba 2025-09-23 6:47 ` Miquel Sabaté Solà 2025-09-23 7:00 ` David Sterba 2025-09-23 8:00 ` Miquel Sabaté Solà 2025-09-19 14:58 ` [PATCH 2/2] btrfs: Prefer using the __free cleanup attribute Miquel Sabaté Solà 2025-09-22 10:34 ` [PATCH 0/2] btrfs: Prevent open-coded arithmetic on kmalloc David Sterba 2025-09-22 12:51 ` Miquel Sabaté Solà 2025-09-23 6:11 ` David Sterba 2025-09-23 6:46 ` Miquel Sabaté Solà
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).