Under the conditions that a device is to be reinitialized within
ufshcd_probe_hba, the device must first be fully reset.
Resetting the device should include freeing U8 model (member of
dev_info) but does not, and this causes a memory leak.
ufs_put_device_desc is responsible for freeing model.
unreferenced object 0xffff3f63008bee60 (size 32):
comm "kworker/u33:1", pid 60, jiffies 4294892642
hex dump (first 32 bytes):
54 48 47 4a 46 47 54 30 54 32 35 42 41 5a 5a 41 THGJFGT0T25BAZZA
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc ed7ff1a9):
[<ffffb86705f1243c>] kmemleak_alloc+0x34/0x40
[<ffffb8670511cee4>] __kmalloc_noprof+0x1e4/0x2fc
[<ffffb86705c247fc>] ufshcd_read_string_desc+0x94/0x190
[<ffffb86705c26854>] ufshcd_device_init+0x480/0xdf8
[<ffffb86705c27b68>] ufshcd_probe_hba+0x3c/0x404
[<ffffb86705c29264>] ufshcd_async_scan+0x40/0x370
[<ffffb86704f43e9c>] async_run_entry_fn+0x34/0xe0
[<ffffb86704f34638>] process_one_work+0x154/0x298
[<ffffb86704f34a74>] worker_thread+0x2f8/0x408
[<ffffb86704f3cfa4>] kthread+0x114/0x118
[<ffffb86704e955a0>] ret_from_fork+0x10/0x20
Signed-off-by: Joel Slebodnick <[email protected]>
---
drivers/ufs/core/ufshcd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0cf07194bbe8..a0407b9213ca 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8787,6 +8787,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
(hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
/* Reset the device and controller before doing reinit */
ufshcd_device_reset(hba);
+ ufs_put_device_desc(hba);
ufshcd_hba_stop(hba);
ufshcd_vops_reinit_notify(hba);
ret = ufshcd_hba_enable(hba);
--
2.40.1
On 6/13/24 11:27 AM, Joel Slebodnick wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 0cf07194bbe8..a0407b9213ca 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8787,6 +8787,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
> (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
> /* Reset the device and controller before doing reinit */
> ufshcd_device_reset(hba);
> + ufs_put_device_desc(hba);
> ufshcd_hba_stop(hba);
> ufshcd_vops_reinit_notify(hba);
> ret = ufshcd_hba_enable(hba);
Please add Fixes: and Cc: stable tags. Otherwise this patch looks good to me.
Hence:
Reviewed-by: Bart Van Assche <[email protected]>
Thanks,
Bart.
On Thu, Jun 13, 2024 at 02:27:28PM GMT, Joel Slebodnick wrote:
> Under the conditions that a device is to be reinitialized within
> ufshcd_probe_hba, the device must first be fully reset.
>
> Resetting the device should include freeing U8 model (member of
> dev_info) but does not, and this causes a memory leak.
> ufs_put_device_desc is responsible for freeing model.
>
> unreferenced object 0xffff3f63008bee60 (size 32):
> comm "kworker/u33:1", pid 60, jiffies 4294892642
> hex dump (first 32 bytes):
> 54 48 47 4a 46 47 54 30 54 32 35 42 41 5a 5a 41 THGJFGT0T25BAZZA
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc ed7ff1a9):
> [<ffffb86705f1243c>] kmemleak_alloc+0x34/0x40
> [<ffffb8670511cee4>] __kmalloc_noprof+0x1e4/0x2fc
> [<ffffb86705c247fc>] ufshcd_read_string_desc+0x94/0x190
> [<ffffb86705c26854>] ufshcd_device_init+0x480/0xdf8
> [<ffffb86705c27b68>] ufshcd_probe_hba+0x3c/0x404
> [<ffffb86705c29264>] ufshcd_async_scan+0x40/0x370
> [<ffffb86704f43e9c>] async_run_entry_fn+0x34/0xe0
> [<ffffb86704f34638>] process_one_work+0x154/0x298
> [<ffffb86704f34a74>] worker_thread+0x2f8/0x408
> [<ffffb86704f3cfa4>] kthread+0x114/0x118
> [<ffffb86704e955a0>] ret_from_fork+0x10/0x20
>
With the following in place:
Fixes: 96a7141da332 ("scsi: ufs: core: Add support for reinitializing the UFS device")
Cc: [email protected]
feel free to add:
Reviewed-by: Andrew Halaney <[email protected]>
> Signed-off-by: Joel Slebodnick <[email protected]>
> ---
> drivers/ufs/core/ufshcd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 0cf07194bbe8..a0407b9213ca 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8787,6 +8787,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
> (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
> /* Reset the device and controller before doing reinit */
> ufshcd_device_reset(hba);
> + ufs_put_device_desc(hba);
> ufshcd_hba_stop(hba);
> ufshcd_vops_reinit_notify(hba);
> ret = ufshcd_hba_enable(hba);
> --
> 2.40.1
>
Under the conditions that a device is to be reinitialized within
ufshcd_probe_hba, the device must first be fully reset.
Resetting the device should include freeing U8 model (member of
dev_info) but does not, and this causes a memory leak.
ufs_put_device_desc is responsible for freeing model.
unreferenced object 0xffff3f63008bee60 (size 32):
comm "kworker/u33:1", pid 60, jiffies 4294892642
hex dump (first 32 bytes):
54 48 47 4a 46 47 54 30 54 32 35 42 41 5a 5a 41 THGJFGT0T25BAZZA
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc ed7ff1a9):
[<ffffb86705f1243c>] kmemleak_alloc+0x34/0x40
[<ffffb8670511cee4>] __kmalloc_noprof+0x1e4/0x2fc
[<ffffb86705c247fc>] ufshcd_read_string_desc+0x94/0x190
[<ffffb86705c26854>] ufshcd_device_init+0x480/0xdf8
[<ffffb86705c27b68>] ufshcd_probe_hba+0x3c/0x404
[<ffffb86705c29264>] ufshcd_async_scan+0x40/0x370
[<ffffb86704f43e9c>] async_run_entry_fn+0x34/0xe0
[<ffffb86704f34638>] process_one_work+0x154/0x298
[<ffffb86704f34a74>] worker_thread+0x2f8/0x408
[<ffffb86704f3cfa4>] kthread+0x114/0x118
[<ffffb86704e955a0>] ret_from_fork+0x10/0x20
Fixes: 96a7141da332 ("scsi: ufs: core: Add support for reinitializing the UFS device")
Cc: <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Signed-off-by: Joel Slebodnick <[email protected]>
---
drivers/ufs/core/ufshcd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0cf07194bbe8..a0407b9213ca 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8787,6 +8787,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
(hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
/* Reset the device and controller before doing reinit */
ufshcd_device_reset(hba);
+ ufs_put_device_desc(hba);
ufshcd_hba_stop(hba);
ufshcd_vops_reinit_notify(hba);
ret = ufshcd_hba_enable(hba);
--
2.40.1
On Thu, 13 Jun 2024 14:27:28 -0400, Joel Slebodnick wrote:
> Under the conditions that a device is to be reinitialized within
> ufshcd_probe_hba, the device must first be fully reset.
>
> Resetting the device should include freeing U8 model (member of
> dev_info) but does not, and this causes a memory leak.
> ufs_put_device_desc is responsible for freeing model.
>
> [...]
Applied to 6.10/scsi-fixes, thanks!
[1/1] scsi: ufs: core: Free memory allocated for model before reinit
https://git.kernel.org/mkp/scsi/c/135c6eb27a85
--
Martin K. Petersen Oracle Linux Engineering
> Under the conditions that a device is to be reinitialized within ufshcd_probe_hba,
> the device must first be fully reset.
>
> Resetting the device should include freeing U8 model (member of
> dev_info) but does not, and this causes a memory leak.
> ufs_put_device_desc is responsible for freeing model.
>
> unreferenced object 0xffff3f63008bee60 (size 32):
> comm "kworker/u33:1", pid 60, jiffies 4294892642
> hex dump (first 32 bytes):
> 54 48 47 4a 46 47 54 30 54 32 35 42 41 5a 5a 41 THGJFGT0T25BAZZA
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc ed7ff1a9):
> [<ffffb86705f1243c>] kmemleak_alloc+0x34/0x40
> [<ffffb8670511cee4>] __kmalloc_noprof+0x1e4/0x2fc
> [<ffffb86705c247fc>] ufshcd_read_string_desc+0x94/0x190
> [<ffffb86705c26854>] ufshcd_device_init+0x480/0xdf8
> [<ffffb86705c27b68>] ufshcd_probe_hba+0x3c/0x404
> [<ffffb86705c29264>] ufshcd_async_scan+0x40/0x370
> [<ffffb86704f43e9c>] async_run_entry_fn+0x34/0xe0
> [<ffffb86704f34638>] process_one_work+0x154/0x298
> [<ffffb86704f34a74>] worker_thread+0x2f8/0x408
> [<ffffb86704f3cfa4>] kthread+0x114/0x118
> [<ffffb86704e955a0>] ret_from_fork+0x10/0x20
>
> Fixes: 96a7141da332 ("scsi: ufs: core: Add support for reinitializing the UFS
> device")
> Cc: <[email protected]>
>
> Reviewed-by: Andrew Halaney <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Signed-off-by: Joel Slebodnick <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
> ---
> drivers/ufs/core/ufshcd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 0cf07194bbe8..a0407b9213ca 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8787,6 +8787,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool
> init_dev_params)
> (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
> /* Reset the device and controller before doing reinit */
> ufshcd_device_reset(hba);
> + ufs_put_device_desc(hba);
> ufshcd_hba_stop(hba);
> ufshcd_vops_reinit_notify(hba);
> ret = ufshcd_hba_enable(hba);
> --
> 2.40.1
On Thu, 2024-06-13 at 16:02 -0400, Joel Slebodnick wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Under the conditions that a device is to be reinitialized within
> ufshcd_probe_hba, the device must first be fully reset.
>
> Resetting the device should include freeing U8 model (member of
> dev_info) but does not, and this causes a memory leak.
> ufs_put_device_desc is responsible for freeing model.
>
> unreferenced object 0xffff3f63008bee60 (size 32):
> comm "kworker/u33:1", pid 60, jiffies 4294892642
> hex dump (first 32 bytes):
> 54 48 47 4a 46 47 54 30 54 32 35 42 41 5a 5a 41 THGJFGT0T25BAZZA
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc ed7ff1a9):
> [<ffffb86705f1243c>] kmemleak_alloc+0x34/0x40
> [<ffffb8670511cee4>] __kmalloc_noprof+0x1e4/0x2fc
> [<ffffb86705c247fc>] ufshcd_read_string_desc+0x94/0x190
> [<ffffb86705c26854>] ufshcd_device_init+0x480/0xdf8
> [<ffffb86705c27b68>] ufshcd_probe_hba+0x3c/0x404
> [<ffffb86705c29264>] ufshcd_async_scan+0x40/0x370
> [<ffffb86704f43e9c>] async_run_entry_fn+0x34/0xe0
> [<ffffb86704f34638>] process_one_work+0x154/0x298
> [<ffffb86704f34a74>] worker_thread+0x2f8/0x408
> [<ffffb86704f3cfa4>] kthread+0x114/0x118
> [<ffffb86704e955a0>] ret_from_fork+0x10/0x20
>
> Fixes: 96a7141da332 ("scsi: ufs: core: Add support for reinitializing
> the UFS device")
> Cc: <[email protected]>
>
> Reviewed-by: Andrew Halaney <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Signed-off-by: Joel Slebodnick <[email protected]>
>
Reviewed-by: Peter Wang <[email protected]>
On Thu, 2024-06-13 at 14:27 -0400, Joel Slebodnick wrote:
> Signed-off-by: Joel Slebodnick <[email protected]>
Reviewed-by: Bean Huo <[email protected]>