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;
Message-ID: <a098244c-fe15-2c88-5c02-59aaa6cc3c6d@gmail.com>
Date:   Tue, 3 Jan 2023 20:16:30 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH] um: vector: Fix memory leak in vector_config
To:     Anton Ivanov <anton.ivanov@cambridgegreys.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Richard Weinberger <richard@nod.at>,
        Johannes Berg <johannes@sipsolutions.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        linux-um@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20221229075323.1394010-1-linmq006@gmail.com>
 <CAMuHMdWFyKA7nUojivvTwWcmxaiUQifRRB=Kn1q-qzmn20Xkuw@mail.gmail.com>
 <eaedf019-349c-c686-0f23-093371b4b61f@cambridgegreys.com>
Content-Language: en-US
From:   Miaoqian Lin <linmq006@gmail.com>
In-Reply-To: <eaedf019-349c-c686-0f23-093371b4b61f@cambridgegreys.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 2023/1/3 18:28, Anton Ivanov wrote:
>
> On 03/01/2023 08:00, Geert Uytterhoeven wrote:
>> Hi Miaoqian,
>>
>> On Thu, Dec 29, 2022 at 8:53 AM Miaoqian Lin <linmq006@gmail.com> wrote:
>>> kstrdup() return newly allocated copy of the string.
>>> Call kfree() to release the memory when after use.
>>>
>>> Fixes: 49da7e64f33e ("High Performance UML Vector Network Driver")
>>> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>> Thanks for your patch!
>>
>>> --- a/arch/um/drivers/vector_kern.c
>>> +++ b/arch/um/drivers/vector_kern.c
>>> @@ -765,6 +765,7 @@ static int vector_config(char *str, char **error_out)
>>>
>>>          parsed = uml_parse_vector_ifspec(params);
>>>
>>> +       kfree(params);
>> Are you sure the memory pointed to by "params" is no longer used?
>> "parsed" seems to contain pointers pointing to (parts of) the string
>> pointed to by "params", so it cannot be freed.
>
> +1.
>
> I was just about to send the same comment.
>
Oh yes, thanks for spotting this. We should only perform release when uml_parse_vector_ifspec() fails (returns NULL). In this situation, 'params' is no longer used. Do you agree?

Thanks,

>>
>>>          if (parsed == NULL) {
>>>                  *error_out = "vector_config failed to parse parameters";
>>>                  return -EINVAL;
>> Gr{oetje,eeting}s,
>>
>>                          Geert
>>
>> -- 
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>>
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like that.
>>                                  -- Linus Torvalds
>>