Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220303081428.12979-1-d.glazkov@omp.ru>
In-Reply-To: <20220303081428.12979-1-d.glazkov@omp.ru>
From:   Dongliang Mu <mudongliangabcd@gmail.com>
Date:   Thu, 3 Mar 2022 20:02:44 +0800
Message-ID: <CAD-N9QUR3H-r4g9jkyDhFnoozVeuUxBvQEMuiBRJ7Q6HHaGsaA@mail.gmail.com>
Subject: Re: [PATCH] KEYS: fix memory leak when reading certificate fails
To:     Denis Glazkov <d.glazkov@omp.ru>
Cc:     David Howells <dhowells@redhat.com>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Mimi Zohar <zohar@linux.vnet.ibm.com>,
        Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
        "linux-security-module@vger.kernel.org" 
        <linux-security-module@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 3, 2022 at 7:49 PM Denis Glazkov <d.glazkov@omp.ru> wrote:
>
> In the `read_file` function of `insert-sys-cert.c` script, if
> the data is read incorrectly, the memory allocated for the `buf`
> array is not freed.
>
> Fixes: c4c361059585 ("KEYS: Reserve an extra certificate symbol for inserting without recompiling")
> Signed-off-by: Denis Glazkov <d.glazkov@omp.ru>
> ---
>  scripts/insert-sys-cert.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
> index 8902836c2342..b98a0b12f16f 100644
> --- a/scripts/insert-sys-cert.c
> +++ b/scripts/insert-sys-cert.c
> @@ -251,6 +251,7 @@ static char *read_file(char *file_name, int *size)
>         if (read(fd, buf, *size) != *size) {
>                 perror("File read failed");
>                 close(fd);
> +               free(buf);
>                 return NULL;
>         }
>         close(fd);

Hi Denis,

There is another issue related to variable buf. On the success path,
buf will be assigned to variable cert in the main function. And cert
is not free when the main function exits.

> --
> 2.25.1