On Thu, Jan 02, 2020 at 01:28:37PM -0800, Guenter Roeck wrote:
On Thu, Jan 02, 2020 at 10:01:19PM +0100, Greg Kroah-Hartman wrote:On Wed, Jan 01, 2020 at 06:44:08PM -0800, Guenter Roeck wrote: > Hi, > > I see a number of crashes in the latest v5.4.y-queue; please see below > for details. The problem bisects to commit 54a311c5d3988d ("clk: Fix memory > leak in clk_unregister()"). > > The context suggests recovery from a failed driver probe, and it appears > that the memory is released twice. Interestingly, I don't see the problem > in mainline. > > I would suggest to drop that patch from the stable queue. That does not look right, as you point out, so I will go drop it now. The logic of the clk structure lifetimes seems crazy, messing with krefs and just "knowing" the lifecycle of the other structures seems like a problem just waiting to happen...I agree. While the patch itself seems to be ok per Stephen's feedback, we have to assume that there will be more secondary failures in addition to the one I have discovered. Given that clocks are not normally unregistered, I don't think fixing the memory leak is important enough to risk the stability of stable releases. With all that in mind, I'd rather have this in mainline for a prolonged period of time before considering it for stable release (if at all).
I would very much like to circle back and add both this patch and it's fix to the stable trees at some point in the future. If the code is good enough for mainline it should be good enough for stable as well. If it's broken - let's fix it now instead of deferring this to when people try to upgrade their major kernel versions. -- Thanks, Sasha
