Re: [PATCH] squashfs: Improve exception handling in squashfs_decompressor_create()

[Markus Elfring]

> Date: Thu, 30 Mar 2023 18:03:47 +0200
>
> The label “out” was used to jump to a kfree() call despite of
> the detail in the implementation of the function
> “squashfs_decompressor_create” that it was determined already
> that a corresponding variable contained a null pointer because of
> a failed memory allocation.
>
> Thus perform the following adjustments:
>
> 1. Return directly after a call of the function “kzalloc” failed
>    at the beginning.
>
> 2. Use more appropriate labels instead.
>
> 3. Omit extra initialisations (for the variables “decomp_strm” and “err”)
>    which became unnecessary with this refactoring.
>
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  fs/squashfs/decompressor_multi.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/fs/squashfs/decompressor_multi.c b/fs/squashfs/decompressor_multi.c
> index 416c53eedbd1..0a054ba4c541 100644
> --- a/fs/squashfs/decompressor_multi.c
> +++ b/fs/squashfs/decompressor_multi.c
> @@ -62,12 +62,12 @@ static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk,
>  				void *comp_opts)
>  {
>  	struct squashfs_stream *stream;
> -	struct decomp_stream *decomp_strm = NULL;
> -	int err = -ENOMEM;
> +	struct decomp_stream *decomp_strm;
> +	int err;
>
>  	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
>  	if (!stream)
> -		goto out;
> +		return ERR_PTR(-ENOMEM);
>
>  	stream->comp_opts = comp_opts;
>  	mutex_init(&stream->mutex);
> @@ -81,22 +81,25 @@ static void *squashfs_decompressor_create(struct squashfs_sb_info *msblk,
>  	 * file system works.
>  	 */
>  	decomp_strm = kmalloc(sizeof(*decomp_strm), GFP_KERNEL);
> -	if (!decomp_strm)
> -		goto out;
> +	if (!decomp_strm) {
> +		err = -ENOMEM;
> +		goto free_stream;
> +	}
>
>  	decomp_strm->stream = msblk->decompressor->init(msblk,
>  						stream->comp_opts);
>  	if (IS_ERR(decomp_strm->stream)) {
>  		err = PTR_ERR(decomp_strm->stream);
> -		goto out;
> +		goto free_decomp_stream;
>  	}
>
>  	list_add(&decomp_strm->list, &stream->strm_list);
>  	stream->avail_decomp = 1;
>  	return stream;
>
> -out:
> +free_decomp_stream:
>  	kfree(decomp_strm);
> +free_stream:
>  	kfree(stream);
>  	return ERR_PTR(err);
>  }

Is this patch still in review queues?

See also:
https://lore.kernel.org/cocci/f1712777-97ff-d89c-0bdd-d72faed9a7f1@xxxxxx/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00120.html

Regards,
Markus