| Message ID | 20220623121857.886-1-nuno.sa@analog.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Dynamic OF and use after free related fixes | expand |
On Thu, 2022-06-23 at 14:18 +0200, Nuno Sá wrote: > This series is a collection of fixes with things I encountered when > handling a system with device tree overlays "fun". This effectively > means > removing and adding devices which led to me the found issues. > > patch 1 is fairly straight and indeed an issue related with of_nodes > not > being put. > > For patches 2 and 3, they are only triggered if someone first removes > the provider which is a dumb thing to do. However, I guess we should > be > prepared for that and the framework even has some efforts to deal > with > these cases (eg: clk_nodrv_ops). That said, patch 2 is more or less > straight (even though catching the guilty line wasn't), we add > something > to a list during resgister, we need to remove it on the unregister > path. > Patch 3 is a different beast... though I think krefs are the proper > solution here, I'm not sure I covered all the corner cases with all > the > reparenting that might happen (mainly reason why this is RFC). > Nullyfing > clk_core->parent during unregister might suffice but I'm more in > favor > of proper refcounting (for now at least). Patch 3 also does not have > a > fixes tag because I genuily don't know when this became a problem. > Maybe > right from the beginning? > > Patch 4 is just a minor improvement and not really a fix... > > v2: > * add an helper function in '__set_clk_parents()' to set each parent > separately; > * massaged commit message in patch 2/4 > ("clk: fix clk not being unlinked from consumers list") as per > Stephen > feedback. > > Nuno Sá (4): > clk: clk-conf: properly release of nodes > clk: fix clk not being unlinked from consumers list > clk: refcount the active parent clk_core > clk: use clk_core_unlink_consumer() helper > > drivers/clk/clk-conf.c | 126 +++++++++++++++++++++++++-------------- > -- > drivers/clk/clk.c | 83 +++++++++++++++++---------- > 2 files changed, 131 insertions(+), 78 deletions(-) > Hi Michael and Stephen, Is there anything missing on this one? I understand that maintainers are very busy people but this is already a RESEND and I already waited a fair amount of time before deciding for this mail. Let me know if there's anything for me to do or if the series is just something you are not keen to apply (even though I think the series makes sense and makes the code more robust). - Nuno Sá
On Wed, 2022-07-13 at 15:24 +0200, Nuno Sá wrote: > On Thu, 2022-06-23 at 14:18 +0200, Nuno Sá wrote: > > This series is a collection of fixes with things I encountered when > > handling a system with device tree overlays "fun". This effectively > > means > > removing and adding devices which led to me the found issues. > > > > patch 1 is fairly straight and indeed an issue related with > > of_nodes > > not > > being put. > > > > For patches 2 and 3, they are only triggered if someone first > > removes > > the provider which is a dumb thing to do. However, I guess we > > should > > be > > prepared for that and the framework even has some efforts to deal > > with > > these cases (eg: clk_nodrv_ops). That said, patch 2 is more or less > > straight (even though catching the guilty line wasn't), we add > > something > > to a list during resgister, we need to remove it on the unregister > > path. > > Patch 3 is a different beast... though I think krefs are the proper > > solution here, I'm not sure I covered all the corner cases with all > > the > > reparenting that might happen (mainly reason why this is RFC). > > Nullyfing > > clk_core->parent during unregister might suffice but I'm more in > > favor > > of proper refcounting (for now at least). Patch 3 also does not > > have > > a > > fixes tag because I genuily don't know when this became a problem. > > Maybe > > right from the beginning? > > > > Patch 4 is just a minor improvement and not really a fix... > > > > v2: > > * add an helper function in '__set_clk_parents()' to set each > > parent > > separately; > > * massaged commit message in patch 2/4 > > ("clk: fix clk not being unlinked from consumers list") as per > > Stephen > > feedback. > > > > Nuno Sá (4): > > clk: clk-conf: properly release of nodes > > clk: fix clk not being unlinked from consumers list > > clk: refcount the active parent clk_core > > clk: use clk_core_unlink_consumer() helper > > > > drivers/clk/clk-conf.c | 126 +++++++++++++++++++++++++------------ > > -- > > -- > > drivers/clk/clk.c | 83 +++++++++++++++++---------- > > 2 files changed, 131 insertions(+), 78 deletions(-) > > > > Hi Michael and Stephen, > > Is there anything missing on this one? I understand that maintainers > are very busy people but this is already a RESEND and I already > waited > a fair amount of time before deciding for this mail. > > Let me know if there's anything for me to do or if the series is just > something you are not keen to apply (even though I think the series > makes sense and makes the code more robust). > > - Nuno Sá Pinging this one more time... I'm happy to rebase and resend but it would be nice to have some feedback. Otherwise I'll just forget about this which is, honesty, very sad since I did spent some time on it :) and it is fixing some real bugs (even though in reality are very hard to trigger). - Nuno Sá
This series is a collection of fixes with things I encountered when handling a system with device tree overlays "fun". This effectively means removing and adding devices which led to me the found issues. patch 1 is fairly straight and indeed an issue related with of_nodes not being put. For patches 2 and 3, they are only triggered if someone first removes the provider which is a dumb thing to do. However, I guess we should be prepared for that and the framework even has some efforts to deal with these cases (eg: clk_nodrv_ops). That said, patch 2 is more or less straight (even though catching the guilty line wasn't), we add something to a list during resgister, we need to remove it on the unregister path. Patch 3 is a different beast... though I think krefs are the proper solution here, I'm not sure I covered all the corner cases with all the reparenting that might happen (mainly reason why this is RFC). Nullyfing clk_core->parent during unregister might suffice but I'm more in favor of proper refcounting (for now at least). Patch 3 also does not have a fixes tag because I genuily don't know when this became a problem. Maybe right from the beginning? Patch 4 is just a minor improvement and not really a fix... v2: * add an helper function in '__set_clk_parents()' to set each parent separately; * massaged commit message in patch 2/4 ("clk: fix clk not being unlinked from consumers list") as per Stephen feedback. Nuno Sá (4): clk: clk-conf: properly release of nodes clk: fix clk not being unlinked from consumers list clk: refcount the active parent clk_core clk: use clk_core_unlink_consumer() helper drivers/clk/clk-conf.c | 126 +++++++++++++++++++++++++---------------- drivers/clk/clk.c | 83 +++++++++++++++++---------- 2 files changed, 131 insertions(+), 78 deletions(-)