From: Stephen Boyd Date: Fri, 18 Apr 2014 23:29:42 +0000 (-0700) Subject: clk: Fix double free due to devm_clk_register() X-Git-Tag: v3.14.6~20 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=798ad2abc1b05842b7bd5292ff43055e4a767a64;p=platform%2Fkernel%2Flinux-stable.git clk: Fix double free due to devm_clk_register() commit 293ba3b4a4fd54891b900f2911d1a57e1ed4a843 upstream. Now that clk_unregister() frees the struct clk we're unregistering we'll free memory twice: first we'll call kfree() in __clk_release() with an address kmalloc doesn't know about and second we'll call kfree() in the devres layer. Remove the allocation of struct clk in devm_clk_register() and let clk_release() handle it. This fixes slab errors like: ============================================================================= BUG kmalloc-128 (Not tainted): Invalid object pointer 0xed08e8d0 ----------------------------------------------------------------------------- Disabling lock debugging due to kernel taint INFO: Slab 0xeec503f8 objects=25 used=15 fp=0xed08ea00 flags=0x4081 CPU: 2 PID: 73 Comm: rmmod Tainted: G B 3.14.0-11032-g526e9c764381 #34 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x70/0xbc) [] (dump_stack) from [] (slab_err+0x74/0x84) [] (slab_err) from [] (free_debug_processing+0x2cc/0x31c) [] (free_debug_processing) from [] (__slab_free+0x38/0x41c) [] (__slab_free) from [] (clk_unregister+0xd4/0x140) [] (clk_unregister) from [] (release_nodes+0x164/0x1d8) [] (release_nodes) from [] (__device_release_driver+0x60/0xb0) [] (__device_release_driver) from [] (driver_detach+0xb4/0xb8) [] (driver_detach) from [] (bus_remove_driver+0x5c/0xc4) [] (bus_remove_driver) from [] (SyS_delete_module+0x148/0x1d8) [] (SyS_delete_module) from [] (ret_fast_syscall+0x0/0x48) FIX kmalloc-128: Object at 0xed08e8d0 not freed Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) Cc: Jiada Wang Cc: Sylwester Nawrocki Cc: Kyungmin Park Signed-off-by: Stephen Boyd Signed-off-by: Mike Turquette Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c42e608..29b909e 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1977,9 +1977,28 @@ struct clk *__clk_register(struct device *dev, struct clk_hw *hw) } EXPORT_SYMBOL_GPL(__clk_register); -static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk) +/** + * clk_register - allocate a new clock, register it and return an opaque cookie + * @dev: device that is registering this clock + * @hw: link to hardware-specific clock data + * + * clk_register is the primary interface for populating the clock tree with new + * clock nodes. It returns a pointer to the newly allocated struct clk which + * cannot be dereferenced by driver code but may be used in conjuction with the + * rest of the clock API. In the event of an error clk_register will return an + * error code; drivers must test for an error code after calling clk_register. + */ +struct clk *clk_register(struct device *dev, struct clk_hw *hw) { int i, ret; + struct clk *clk; + + clk = kzalloc(sizeof(*clk), GFP_KERNEL); + if (!clk) { + pr_err("%s: could not allocate clk\n", __func__); + ret = -ENOMEM; + goto fail_out; + } clk->name = kstrdup(hw->init->name, GFP_KERNEL); if (!clk->name) { @@ -2019,7 +2038,7 @@ static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk) ret = __clk_init(dev, clk); if (!ret) - return 0; + return clk; fail_parent_names_copy: while (--i >= 0) @@ -2028,36 +2047,6 @@ fail_parent_names_copy: fail_parent_names: kfree(clk->name); fail_name: - return ret; -} - -/** - * clk_register - allocate a new clock, register it and return an opaque cookie - * @dev: device that is registering this clock - * @hw: link to hardware-specific clock data - * - * clk_register is the primary interface for populating the clock tree with new - * clock nodes. It returns a pointer to the newly allocated struct clk which - * cannot be dereferenced by driver code but may be used in conjuction with the - * rest of the clock API. In the event of an error clk_register will return an - * error code; drivers must test for an error code after calling clk_register. - */ -struct clk *clk_register(struct device *dev, struct clk_hw *hw) -{ - int ret; - struct clk *clk; - - clk = kzalloc(sizeof(*clk), GFP_KERNEL); - if (!clk) { - pr_err("%s: could not allocate clk\n", __func__); - ret = -ENOMEM; - goto fail_out; - } - - ret = _clk_register(dev, hw, clk); - if (!ret) - return clk; - kfree(clk); fail_out: return ERR_PTR(ret); @@ -2166,7 +2155,7 @@ EXPORT_SYMBOL_GPL(clk_unregister); static void devm_clk_release(struct device *dev, void *res) { - clk_unregister(res); + clk_unregister(*(struct clk **)res); } /** @@ -2181,18 +2170,18 @@ static void devm_clk_release(struct device *dev, void *res) struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw) { struct clk *clk; - int ret; + struct clk **clkp; - clk = devres_alloc(devm_clk_release, sizeof(*clk), GFP_KERNEL); - if (!clk) + clkp = devres_alloc(devm_clk_release, sizeof(*clkp), GFP_KERNEL); + if (!clkp) return ERR_PTR(-ENOMEM); - ret = _clk_register(dev, hw, clk); - if (!ret) { - devres_add(dev, clk); + clk = clk_register(dev, hw); + if (!IS_ERR(clk)) { + *clkp = clk; + devres_add(dev, clkp); } else { - devres_free(clk); - clk = ERR_PTR(ret); + devres_free(clkp); } return clk;