[RESEND PATCH] memstick: fix memory leak if card device is never registered

[RESEND PATCH] memstick: fix memory leak if card device is never registered

[Greg Kroah-Hartman]

When calling dev_set_name() memory is allocated for the name for the
struct device.  Once that structure device is registered, or attempted
to be registerd, with the driver core, the driver core will handle
cleaning up that memory when the device is removed from the system.

Unfortunatly for the memstick code, there is an error path that causes
the struct device to never be registered, and so the memory allocated in
dev_set_name will be leaked.  Fix that leak by manually freeing it right
before the memory for the device is freed.

Cc: Maxim Levitsky <maximlevitsky@gmail.com>
Cc: Alex Dubov <oakad@yahoo.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: linux-mmc@vger.kernel.org
Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
Cc: stable <stable@kernel.org>
Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
---
RESEND as the first version had a corrupted message id and never made it
to the mailing lists or lore.kernel.org

 drivers/memstick/core/memstick.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index bf7667845459..bbfaf6536903 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
 	return card;
 err_out:
 	host->card = old_card;
+	kfree_const(card->dev.kobj.name);
 	kfree(card);
 	return NULL;
 }
@@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work)
 				put_device(&card->dev);
 				host->card = NULL;
 			}
-		} else
+		} else {
+			kfree_const(card->dev.kobj.name);
 			kfree(card);
+		}
 	}
 
 out_power_off:
-- 
2.40.0

Re: [RESEND PATCH] memstick: fix memory leak if card device is never registered

[Mirsad Goran Todorovac]

On 1.4.2023. 22:03, Greg Kroah-Hartman wrote:
> When calling dev_set_name() memory is allocated for the name for the
> struct device.  Once that structure device is registered, or attempted
> to be registerd, with the driver core, the driver core will handle
> cleaning up that memory when the device is removed from the system.
> 
> Unfortunatly for the memstick code, there is an error path that causes
> the struct device to never be registered, and so the memory allocated in
> dev_set_name will be leaked.  Fix that leak by manually freeing it right
> before the memory for the device is freed.
> 
> Cc: Maxim Levitsky <maximlevitsky@gmail.com>
> Cc: Alex Dubov <oakad@yahoo.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Kay Sievers <kay.sievers@vrfy.org>
> Cc: linux-mmc@vger.kernel.org
> Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
> Cc: stable <stable@kernel.org>
> Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> ---
> RESEND as the first version had a corrupted message id and never made it
> to the mailing lists or lore.kernel.org
> 
>   drivers/memstick/core/memstick.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
> index bf7667845459..bbfaf6536903 100644
> --- a/drivers/memstick/core/memstick.c
> +++ b/drivers/memstick/core/memstick.c
> @@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
>   	return card;
>   err_out:
>   	host->card = old_card;
> +	kfree_const(card->dev.kobj.name);
>   	kfree(card);
>   	return NULL;
>   }
> @@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work)
>   				put_device(&card->dev);
>   				host->card = NULL;
>   			}
> -		} else
> +		} else {
> +			kfree_const(card->dev.kobj.name);
>   			kfree(card);
> +		}
>   	}
>   
>   out_power_off:

FYI, the applied patch tested OK, no memstick leaks:

[root@pc-mtodorov kernel]# uname -rms
Linux 6.3.0-rc5-mt-20230401-00007-g268a637be362 x86_64
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak

What was applied is:

mtodorov@domac:~/linux/kernel/linux_torvalds$ git diff master..origin/master | head -24
diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index bbfaf6536903..bf7667845459 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -410,7 +410,6 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
         return card;
  err_out:
         host->card = old_card;
-       kfree_const(card->dev.kobj.name);
         kfree(card);
         return NULL;
  }
@@ -469,10 +468,8 @@ static void memstick_check(struct work_struct *work)
                                 put_device(&card->dev);
                                 host->card = NULL;
                         }
-               } else {
-                       kfree_const(card->dev.kobj.name);
+               } else
                         kfree(card);
-               }
         }

  out_power_off:

APPENDIX

However, please note that the patch SHA-1's truncated to 12 chars might not
be the same on the other systems, so the build ID says nothing about which
mainline kernel had the patches been applied against. So `uname -rms` is
telling pretty nothing about which kernel I'm running, except that it helps
me distinguish between the builds.

Nothing to prove that:

- I am not testing the wrong kernel and
- that the right patches have been applied.

Changing CONFIG_LOCALVERSION causes copious recompilations, even with ccache
it takes about 4x the time needed to compile CONFIG_LOCALVERSION_AUTO=y.

Do I make any sense with this?

I am adding Cc: to Mr. Bagas, for we spoke already about kernel versioning
in the case of manually applied patches.

Regards,
Mirsad

-- 
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union

Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu

Re: [RESEND PATCH] memstick: fix memory leak if card device is never registered

[Ulf Hansson]

On Sat, 1 Apr 2023 at 22:03, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> When calling dev_set_name() memory is allocated for the name for the
> struct device.  Once that structure device is registered, or attempted
> to be registerd, with the driver core, the driver core will handle
> cleaning up that memory when the device is removed from the system.
>
> Unfortunatly for the memstick code, there is an error path that causes
> the struct device to never be registered, and so the memory allocated in
> dev_set_name will be leaked.  Fix that leak by manually freeing it right
> before the memory for the device is freed.
>
> Cc: Maxim Levitsky <maximlevitsky@gmail.com>
> Cc: Alex Dubov <oakad@yahoo.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Kay Sievers <kay.sievers@vrfy.org>
> Cc: linux-mmc@vger.kernel.org
> Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
> Cc: stable <stable@kernel.org>
> Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>

Applied for fixes and by adding Mirsad's sob tag (according to the
other thread [1]), thanks!

Kind regards
Uffe

[1]
https://lore.kernel.org/lkml/c059f486-98a6-aecd-c135-c033612e6b4f@alu.unizg.hr/


> ---
> RESEND as the first version had a corrupted message id and never made it
> to the mailing lists or lore.kernel.org
>
>  drivers/memstick/core/memstick.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
> index bf7667845459..bbfaf6536903 100644
> --- a/drivers/memstick/core/memstick.c
> +++ b/drivers/memstick/core/memstick.c
> @@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
>         return card;
>  err_out:
>         host->card = old_card;
> +       kfree_const(card->dev.kobj.name);
>         kfree(card);
>         return NULL;
>  }
> @@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work)
>                                 put_device(&card->dev);
>                                 host->card = NULL;
>                         }
> -               } else
> +               } else {
> +                       kfree_const(card->dev.kobj.name);
>                         kfree(card);
> +               }
>         }
>
>  out_power_off:
> --
> 2.40.0
>

Re: [RESEND PATCH] memstick: fix memory leak if card device is never registered

[Greg Kroah-Hartman]

On Tue, Apr 04, 2023 at 01:54:03PM +0200, Ulf Hansson wrote:
> On Sat, 1 Apr 2023 at 22:03, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > When calling dev_set_name() memory is allocated for the name for the
> > struct device.  Once that structure device is registered, or attempted
> > to be registerd, with the driver core, the driver core will handle
> > cleaning up that memory when the device is removed from the system.
> >
> > Unfortunatly for the memstick code, there is an error path that causes
> > the struct device to never be registered, and so the memory allocated in
> > dev_set_name will be leaked.  Fix that leak by manually freeing it right
> > before the memory for the device is freed.
> >
> > Cc: Maxim Levitsky <maximlevitsky@gmail.com>
> > Cc: Alex Dubov <oakad@yahoo.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Kay Sievers <kay.sievers@vrfy.org>
> > Cc: linux-mmc@vger.kernel.org
> > Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
> > Cc: stable <stable@kernel.org>
> > Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> 
> Applied for fixes and by adding Mirsad's sob tag (according to the
> other thread [1]), thanks!

Wonderful, thanks for picking up that tag, and the patch.

greg k-h