memstick: fix memory leak if card device is never registered
Subsystems (3)
PATCH RESEND 1/1 memstick: fix memory leak if card device is never registered
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.
---
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(-)
drivers/memstick/core/memstick.c +4 -1
@@ -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:
Signature · 1 line
2.40.0
Quoted 43 lines · 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_namewill be leaked. Fix that leak by manually freeing it right before the memory for the device is freed.Cc Maxim Levitsky <[email protected]>Cc Alex Dubov <[email protected]>Cc Ulf Hansson <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Hans de Goede <[email protected]>Cc Kay Sievers <[email protected]>Fixes 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),Cc stable <[email protected]>Co-developed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>Co-developed-by Mirsad Goran Todorovac <[email protected]>---
RESEND as the first version had a corrupted message id and never made it to the mailing lists or lore.kernel.orgdrivers/memstick/core/memstick.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)drivers/memstick/core/memstick.c +1
-0@@ -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);
drivers/memstick/core/memstick.c static void memstick_check(struct work_struct *work) · line 478
Quoted 10 lines
drivers/memstick/core/memstick.c +3
-1host->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
drivers/memstick/core/memstick.c +1 -4
@@ -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
Signature · 9 lines
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
Quoted 11 lines · 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_namewill be leaked. Fix that leak by manually freeing it right before the memory for the device is freed.
Quoted 12 lines
Cc Maxim Levitsky <[email protected]>Cc Alex Dubov <[email protected]>Cc Ulf Hansson <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Hans de Goede <[email protected]>Cc Kay Sievers <[email protected]>Fixes 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),Cc stable <[email protected]>Co-developed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>Co-developed-by Mirsad Goran Todorovac <[email protected]>
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/[email protected]/
Quoted 34 lines
---
RESEND as the first version had a corrupted message id and never made it to the mailing lists or lore.kernel.orgdrivers/memstick/core/memstick.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)drivers/memstick/core/memstick.c +4
-2@@ -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
Quoted 26 lines · Ulf Hansson
Quoted 11 lines · 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_namewill be leaked. Fix that leak by manually freeing it right before the memory for the device is freed.Quoted 12 lines
Cc Maxim Levitsky <[email protected]>Cc Alex Dubov <[email protected]>Cc Ulf Hansson <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Hans de Goede <[email protected]>Cc Kay Sievers <[email protected]>Fixes 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),Cc stable <[email protected]>Co-developed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>Co-developed-by Mirsad Goran Todorovac <[email protected]>
Quoted 2 lines
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