Re: [PATCH v2] btrfs: Remove open-coded arithmetic in kmalloc

["Miquel Sabaté Solà" <mssola@mssola.com>]

[-- Attachment #1: Type: text/plain, Size: 2954 bytes --]

Miquel Sabaté Solà @ 2025-09-23 09:56 +02:

> This is an API cleanup in which the deprecated use of 'kmalloc' with
> open-coded arithmetic is being removed in favor of 'kmalloc_array'. This
> doesn't fix any overflow we are currently facing as all multipliers are
> bounded small numbers derived from number of items in leaves/nodes, but
> it's still a good idea to move away from deprecated uses of 'kmalloc'.
>
> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>
> ---
>
> Changes in v2:
> - Provide better wording since this is not fixing any current overflow
>   issues.
> - Drop commit introducing some new __free(kfree) uses in favor of a
>   new patch set to be provided in the future which does a more
>   systematic change.
>
>  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;

As discussed with David Sterba, you can ignore this one as it has
already been addressed on his side.

Sorry for the noise.
Miquel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]