| Message ID | 20240613182728.2521951-1-jslebodn@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: ufs: core: Free memory allocated for model before reinit | expand |
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 <bvanassche@acm.org> 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: stable@vger.kernel.org feel free to add: Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > Signed-off-by: Joel Slebodnick <jslebodn@redhat.com> > --- > 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
On Thu, 2024-06-13 at 14:27 -0400, Joel Slebodnick wrote: > Signed-off-by: Joel Slebodnick <jslebodn@redhat.com> Reviewed-by: Bean Huo <beanhuo@micron.com>
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);
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 <jslebodn@redhat.com> --- drivers/ufs/core/ufshcd.c | 1 + 1 file changed, 1 insertion(+)