All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] um: vector: Fix memory leak in vector_config
@ 2022-12-29  7:53 ` Miaoqian Lin
  0 siblings, 0 replies; 10+ messages in thread
From: Miaoqian Lin @ 2022-12-29  7:53 UTC (permalink / raw)
  To: Richard Weinberger, Anton Ivanov, Johannes Berg, Jakub Kicinski,
	Miaoqian Lin, David S. Miller, Wolfram Sang, linux-um,
	linux-kernel

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>
---
 arch/um/drivers/vector_kern.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index ded7c47d2fbe..78f32005dd90 100644
--- 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);
 	if (parsed == NULL) {
 		*error_out = "vector_config failed to parse parameters";
 		return -EINVAL;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] um: vector: Fix memory leak in vector_config
@ 2022-12-29  7:53 ` Miaoqian Lin
  0 siblings, 0 replies; 10+ messages in thread
From: Miaoqian Lin @ 2022-12-29  7:53 UTC (permalink / raw)
  To: Richard Weinberger, Anton Ivanov, Johannes Berg, Jakub Kicinski,
	Miaoqian Lin, David S. Miller, Wolfram Sang, linux-um,
	linux-kernel

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>
---
 arch/um/drivers/vector_kern.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index ded7c47d2fbe..78f32005dd90 100644
--- 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);
 	if (parsed == NULL) {
 		*error_out = "vector_config failed to parse parameters";
 		return -EINVAL;
-- 
2.25.1


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
  2022-12-29  7:53 ` Miaoqian Lin
@ 2023-01-03  8:00   ` Geert Uytterhoeven
  -1 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2023-01-03  8:00 UTC (permalink / raw)
  To: Miaoqian Lin
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel

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.

>         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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
@ 2023-01-03  8:00   ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2023-01-03  8:00 UTC (permalink / raw)
  To: Miaoqian Lin
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel

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.

>         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

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
  2023-01-03  8:00   ` Geert Uytterhoeven
@ 2023-01-03 10:28     ` Anton Ivanov
  -1 siblings, 0 replies; 10+ messages in thread
From: Anton Ivanov @ 2023-01-03 10:28 UTC (permalink / raw)
  To: Geert Uytterhoeven, Miaoqian Lin
  Cc: Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel


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.

>
>>          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
>
-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
@ 2023-01-03 10:28     ` Anton Ivanov
  0 siblings, 0 replies; 10+ messages in thread
From: Anton Ivanov @ 2023-01-03 10:28 UTC (permalink / raw)
  To: Geert Uytterhoeven, Miaoqian Lin
  Cc: Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel


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.

>
>>          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
>
-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
  2023-01-03 10:28     ` Anton Ivanov
@ 2023-01-03 12:16       ` Miaoqian Lin
  -1 siblings, 0 replies; 10+ messages in thread
From: Miaoqian Lin @ 2023-01-03 12:16 UTC (permalink / raw)
  To: Anton Ivanov, Geert Uytterhoeven
  Cc: Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel


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
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
@ 2023-01-03 12:16       ` Miaoqian Lin
  0 siblings, 0 replies; 10+ messages in thread
From: Miaoqian Lin @ 2023-01-03 12:16 UTC (permalink / raw)
  To: Anton Ivanov, Geert Uytterhoeven
  Cc: Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel


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
>>

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
  2023-01-03 12:16       ` Miaoqian Lin
@ 2023-01-03 13:06         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2023-01-03 13:06 UTC (permalink / raw)
  To: Miaoqian Lin
  Cc: Anton Ivanov, Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel

Hi Miaoqian,

On Tue, Jan 3, 2023 at 1:17 PM Miaoqian Lin <linmq006@gmail.com> wrote:
> On 2023/1/3 18:28, Anton Ivanov wrote:
> > On 03/01/2023 08:00, Geert Uytterhoeven wrote:
> >> 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?

Yes, that sounds fine to me.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] um: vector: Fix memory leak in vector_config
@ 2023-01-03 13:06         ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2023-01-03 13:06 UTC (permalink / raw)
  To: Miaoqian Lin
  Cc: Anton Ivanov, Richard Weinberger, Johannes Berg, Jakub Kicinski,
	David S. Miller, Wolfram Sang, linux-um, linux-kernel

Hi Miaoqian,

On Tue, Jan 3, 2023 at 1:17 PM Miaoqian Lin <linmq006@gmail.com> wrote:
> On 2023/1/3 18:28, Anton Ivanov wrote:
> > On 03/01/2023 08:00, Geert Uytterhoeven wrote:
> >> 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?

Yes, that sounds fine to me.

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

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-01-03 16:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29  7:53 [PATCH] um: vector: Fix memory leak in vector_config Miaoqian Lin
2022-12-29  7:53 ` Miaoqian Lin
2023-01-03  8:00 ` Geert Uytterhoeven
2023-01-03  8:00   ` Geert Uytterhoeven
2023-01-03 10:28   ` Anton Ivanov
2023-01-03 10:28     ` Anton Ivanov
2023-01-03 12:16     ` Miaoqian Lin
2023-01-03 12:16       ` Miaoqian Lin
2023-01-03 13:06       ` Geert Uytterhoeven
2023-01-03 13:06         ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.