I case of partition device_unregister in mtd_device_release
calls mtd_release which frees mtd_info structure for partition.
All code after device_unregister in mtd_device_release thus
works already freed memory.
Move part of code to mtd_release and restict mtd->dev cleanup
to non-partion object.
For partition object such cleanup have no sense as partition
mtd_info is removed.
Cc: Miquel Raynal <[email protected]>
Cc: Zhang Xiaoxu <[email protected]>
Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
Reviewed-by: Tomas Winkler <[email protected]>
Signed-off-by: Alexander Usyskin <[email protected]>
---
drivers/mtd/mtdcore.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 2466ea466466..46f15f676491 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -93,6 +93,9 @@ static void mtd_release(struct device *dev)
struct mtd_info *mtd = dev_get_drvdata(dev);
dev_t index = MTD_DEVT(mtd->index);
+ idr_remove(&mtd_idr, mtd->index);
+ of_node_put(mtd_get_of_node(mtd));
+
if (mtd_is_partition(mtd))
release_mtd_partition(mtd);
@@ -103,6 +106,7 @@ static void mtd_release(struct device *dev)
static void mtd_device_release(struct kref *kref)
{
struct mtd_info *mtd = container_of(kref, struct mtd_info, refcnt);
+ bool is_partition = mtd_is_partition(mtd);
debugfs_remove_recursive(mtd->dbg.dfs_dir);
@@ -111,11 +115,13 @@ static void mtd_device_release(struct kref *kref)
device_unregister(&mtd->dev);
- /* Clear dev so mtd can be safely re-registered later if desired */
- memset(&mtd->dev, 0, sizeof(mtd->dev));
-
- idr_remove(&mtd_idr, mtd->index);
- of_node_put(mtd_get_of_node(mtd));
+ /*
+ * Clear dev so mtd can be safely re-registered later if desired.
+ * Should not be done for partition,
+ * as it was already destroyed in device_unregister().
+ */
+ if (!is_partition)
+ memset(&mtd->dev, 0, sizeof(mtd->dev));
module_put(THIS_MODULE);
}
--
2.34.1
Hi Andy,
[email protected] wrote on Thu, 27 Jul 2023 18:12:04
+0300:
> On Thu, Jul 27, 2023 at 05:57:58PM +0300, Alexander Usyskin wrote:
> > I case of partition device_unregister in mtd_device_release
>
> In
>
> device_unregister()
> mtd_device_release()
>
> > calls mtd_release which frees mtd_info structure for partition.
>
> mtd_release()
>
> > All code after device_unregister in mtd_device_release thus
>
> device_unregister()
> mtd_device_release()
>
> > works already freed memory.
>
> uses?
>
> > Move part of code to mtd_release and restict mtd->dev cleanup
>
> mtd_release()
Yup, thanks for all these suggestions, I agree with them.
> > to non-partion object.
> > For partition object such cleanup have no sense as partition
> > mtd_info is removed.
> >
> > Cc: Miquel Raynal <[email protected]>
> > Cc: Zhang Xiaoxu <[email protected]>
> > Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
>
> Closes: ?
Did I miss a recent update on the use of Fixes? I thought Closes was
supposed to point at a bug report while Fixes would point to the faulty
commit. Right now I feel like Fixes is the right tag, but if you have a
source explaining why we should not longer do it like I am used to,
I would appreciate a link.
Thanks,
Miquèl
On Thu, Jul 27, 2023 at 05:57:58PM +0300, Alexander Usyskin wrote:
> I case of partition device_unregister in mtd_device_release
In
device_unregister()
mtd_device_release()
> calls mtd_release which frees mtd_info structure for partition.
mtd_release()
> All code after device_unregister in mtd_device_release thus
device_unregister()
mtd_device_release()
> works already freed memory.
uses?
> Move part of code to mtd_release and restict mtd->dev cleanup
mtd_release()
> to non-partion object.
> For partition object such cleanup have no sense as partition
> mtd_info is removed.
>
> Cc: Miquel Raynal <[email protected]>
> Cc: Zhang Xiaoxu <[email protected]>
> Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
Closes: ?
--
With Best Regards,
Andy Shevchenko
On Thu, Jul 27, 2023 at 05:20:13PM +0200, Miquel Raynal wrote:
> [email protected] wrote on Thu, 27 Jul 2023 18:12:04
> +0300:
> > On Thu, Jul 27, 2023 at 05:57:58PM +0300, Alexander Usyskin wrote:
...
> > > Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
> >
> > Closes: ?
>
> Did I miss a recent update on the use of Fixes?
They are orthogonal to each other. Actually Closes goes closer with
Reported-by.
I believe both of them needs to be added (by I might miss something).
> I thought Closes was
> supposed to point at a bug report while Fixes would point to the faulty
> commit.
Correct.
> Right now I feel like Fixes is the right tag,
Nobody objects that (see above).
> but if you have a source explaining why we should not longer do it like
> I am used to, I would appreciate a link.
Since you know about Closes already, I think there is nothing to add.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
[email protected] wrote on Thu, 27 Jul 2023 18:58:29
+0300:
> On Thu, Jul 27, 2023 at 05:20:13PM +0200, Miquel Raynal wrote:
> > [email protected] wrote on Thu, 27 Jul 2023 18:12:04
> > +0300:
> > > On Thu, Jul 27, 2023 at 05:57:58PM +0300, Alexander Usyskin wrote:
>
> ...
>
> > > > Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
> > >
> > > Closes: ?
> >
> > Did I miss a recent update on the use of Fixes?
>
> They are orthogonal to each other. Actually Closes goes closer with
> Reported-by.
>
> I believe both of them needs to be added (by I might miss something).
>
> > I thought Closes was
> > supposed to point at a bug report while Fixes would point to the faulty
> > commit.
>
> Correct.
>
> > Right now I feel like Fixes is the right tag,
>
> Nobody objects that (see above).
>
> > but if you have a source explaining why we should not longer do it like
> > I am used to, I would appreciate a link.
>
> Since you know about Closes already, I think there is nothing to add.
Ah sorry I misunderstood your first e-mail. I thought you were
suggesting to replace Fixes by Closes. Sorry for the misunderstanding :)
Thanks,
Miquèl
>
> Hi Andy,
>
> [email protected] wrote on Thu, 27 Jul 2023 18:58:29
> +0300:
>
> > On Thu, Jul 27, 2023 at 05:20:13PM +0200, Miquel Raynal wrote:
> > > [email protected] wrote on Thu, 27 Jul 2023 18:12:04
> > > +0300:
> > > > On Thu, Jul 27, 2023 at 05:57:58PM +0300, Alexander Usyskin wrote:
> >
> > ...
> >
> > > > > Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
> > > >
> > > > Closes: ?
> > >
> > > Did I miss a recent update on the use of Fixes?
> >
> > They are orthogonal to each other. Actually Closes goes closer with
> > Reported-by.
> >
> > I believe both of them needs to be added (by I might miss something).
> >
> > > I thought Closes was
> > > supposed to point at a bug report while Fixes would point to the faulty
> > > commit.
> >
> > Correct.
> >
> > > Right now I feel like Fixes is the right tag,
> >
> > Nobody objects that (see above).
> >
> > > but if you have a source explaining why we should not longer do it like
> > > I am used to, I would appreciate a link.
> >
> > Since you know about Closes already, I think there is nothing to add.
>
> Ah sorry I misunderstood your first e-mail. I thought you were
> suggesting to replace Fixes by Closes. Sorry for the misunderstanding :)
>
> Thanks,
> Miquèl
Miquel, is this patch helps with your original problem of devices not freed?
Zhang, is this patch helps with your problem with KAsan?
--
Thanks,
Sasha
在 2023/7/30 19:10, Usyskin, Alexander 写道:
> Miquel, is this patch helps with your original problem of devices not freed?
>
> Zhang, is this patch helps with your problem with KAsan?
After this patch applied, the problem can still be reproduced.
Hi zhang,
[email protected] wrote on Mon, 31 Jul 2023 09:35:42 +0800:
> 在 2023/7/30 19:10, Usyskin, Alexander 写道:
> > Miquel, is this patch helps with your original problem of devices not freed?
> >
> > Zhang, is this patch helps with your problem with KAsan?
> After this patch applied, the problem can still be reproduced.
Did you test my patch as well? Does Kasan still complain with it?
Thanks,
Miquèl
在 2023/8/2 20:44, Miquel Raynal 写道:
> Hi zhang,
>
> [email protected] wrote on Mon, 31 Jul 2023 09:35:42 +0800:
>
>> 在 2023/7/30 19:10, Usyskin, Alexander 写道:
>>> Miquel, is this patch helps with your original problem of devices not freed?
>>>
>>> Zhang, is this patch helps with your problem with KAsan?
>> After this patch applied, the problem can still be reproduced.
>
> Did you test my patch as well? Does Kasan still complain with it?
After this patch applied, the problem can still be reproduced.
>
> Thanks,
> Miquèl