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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
Cc: Alex Dubov <oakad@xxxxxxxxx>
Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
Cc: Kay Sievers <kay.sievers@xxxxxxxx>
Cc: linux-mmc@xxxxxxxxxxxxxxx
Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
Cc: stable <stable@xxxxxxxxxx>
Co-developed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@xxxxxxxxxxxx>
---
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




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux