2024-01-05 20:32:32

by Markus Elfring

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

> 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 <[email protected]>
> ---
> 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/[email protected]/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00120.html

Regards,
Markus