* [PATCH 0/2] Clock unregistration fixes @ 2014-04-18 23:29 ` Stephen Boyd 0 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: linux-arm-kernel Two fixes for fallout from commit fcb0ee6a3d33 (clk: Implement clk_unregister). Stephen Boyd (2): clk: Fix double free due to devm_clk_register() clk: Fix slab corruption in clk_unregister() drivers/clk/clk.c | 74 ++++++++++++++++++++++++------------------------------- 1 file changed, 32 insertions(+), 42 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/2] Clock unregistration fixes @ 2014-04-18 23:29 ` Stephen Boyd 0 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: Mike Turquette; +Cc: linux-kernel, linux-arm-kernel Two fixes for fallout from commit fcb0ee6a3d33 (clk: Implement clk_unregister). Stephen Boyd (2): clk: Fix double free due to devm_clk_register() clk: Fix slab corruption in clk_unregister() drivers/clk/clk.c | 74 ++++++++++++++++++++++++------------------------------- 1 file changed, 32 insertions(+), 42 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] clk: Fix double free due to devm_clk_register() 2014-04-18 23:29 ` Stephen Boyd @ 2014-04-18 23:29 ` Stephen Boyd -1 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: linux-arm-kernel 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 [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) [<c0012240>] (show_stack) from [<c04b74dc>] (dump_stack+0x70/0xbc) [<c04b74dc>] (dump_stack) from [<c00f6778>] (slab_err+0x74/0x84) [<c00f6778>] (slab_err) from [<c04b6278>] (free_debug_processing+0x2cc/0x31c) [<c04b6278>] (free_debug_processing) from [<c04b6300>] (__slab_free+0x38/0x41c) [<c04b6300>] (__slab_free) from [<c03931bc>] (clk_unregister+0xd4/0x140) [<c03931bc>] (clk_unregister) from [<c02fb774>] (release_nodes+0x164/0x1d8) [<c02fb774>] (release_nodes) from [<c02f8698>] (__device_release_driver+0x60/0xb0) [<c02f8698>] (__device_release_driver) from [<c02f9080>] (driver_detach+0xb4/0xb8) [<c02f9080>] (driver_detach) from [<c02f8480>] (bus_remove_driver+0x5c/0xc4) [<c02f8480>] (bus_remove_driver) from [<c008c9b8>] (SyS_delete_module+0x148/0x1d8) [<c008c9b8>] (SyS_delete_module) from [<c000ef80>] (ret_fast_syscall+0x0/0x48) FIX kmalloc-128: Object at 0xed08e8d0 not freed Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) Cc: Jiada Wang <jiada_wang@mentor.com> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk.c | 71 +++++++++++++++++++++++-------------------------------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index dff0373f53c1..f71093bf83ab 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1984,9 +1984,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) { @@ -2026,7 +2045,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) @@ -2035,36 +2054,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); @@ -2173,7 +2162,7 @@ EXPORT_SYMBOL_GPL(clk_unregister); static void devm_clk_release(struct device *dev, void *res) { - clk_unregister(res); + clk_unregister(*(struct clk **)res); } /** @@ -2188,18 +2177,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; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] clk: Fix double free due to devm_clk_register() @ 2014-04-18 23:29 ` Stephen Boyd 0 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: Mike Turquette Cc: linux-kernel, linux-arm-kernel, Jiada Wang, Sylwester Nawrocki, Kyungmin Park 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 [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) [<c0012240>] (show_stack) from [<c04b74dc>] (dump_stack+0x70/0xbc) [<c04b74dc>] (dump_stack) from [<c00f6778>] (slab_err+0x74/0x84) [<c00f6778>] (slab_err) from [<c04b6278>] (free_debug_processing+0x2cc/0x31c) [<c04b6278>] (free_debug_processing) from [<c04b6300>] (__slab_free+0x38/0x41c) [<c04b6300>] (__slab_free) from [<c03931bc>] (clk_unregister+0xd4/0x140) [<c03931bc>] (clk_unregister) from [<c02fb774>] (release_nodes+0x164/0x1d8) [<c02fb774>] (release_nodes) from [<c02f8698>] (__device_release_driver+0x60/0xb0) [<c02f8698>] (__device_release_driver) from [<c02f9080>] (driver_detach+0xb4/0xb8) [<c02f9080>] (driver_detach) from [<c02f8480>] (bus_remove_driver+0x5c/0xc4) [<c02f8480>] (bus_remove_driver) from [<c008c9b8>] (SyS_delete_module+0x148/0x1d8) [<c008c9b8>] (SyS_delete_module) from [<c000ef80>] (ret_fast_syscall+0x0/0x48) FIX kmalloc-128: Object at 0xed08e8d0 not freed Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) Cc: Jiada Wang <jiada_wang@mentor.com> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk.c | 71 +++++++++++++++++++++++-------------------------------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index dff0373f53c1..f71093bf83ab 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1984,9 +1984,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) { @@ -2026,7 +2045,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) @@ -2035,36 +2054,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); @@ -2173,7 +2162,7 @@ EXPORT_SYMBOL_GPL(clk_unregister); static void devm_clk_release(struct device *dev, void *res) { - clk_unregister(res); + clk_unregister(*(struct clk **)res); } /** @@ -2188,18 +2177,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; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] clk: Fix double free due to devm_clk_register() 2014-04-18 23:29 ` Stephen Boyd @ 2014-05-14 11:26 ` Sylwester Nawrocki -1 siblings, 0 replies; 11+ messages in thread From: Sylwester Nawrocki @ 2014-05-14 11:26 UTC (permalink / raw) To: linux-arm-kernel On 19/04/14 01:29, Stephen Boyd wrote: > 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 > [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) > [<c0012240>] (show_stack) from [<c04b74dc>] (dump_stack+0x70/0xbc) > [<c04b74dc>] (dump_stack) from [<c00f6778>] (slab_err+0x74/0x84) > [<c00f6778>] (slab_err) from [<c04b6278>] (free_debug_processing+0x2cc/0x31c) > [<c04b6278>] (free_debug_processing) from [<c04b6300>] (__slab_free+0x38/0x41c) > [<c04b6300>] (__slab_free) from [<c03931bc>] (clk_unregister+0xd4/0x140) > [<c03931bc>] (clk_unregister) from [<c02fb774>] (release_nodes+0x164/0x1d8) > [<c02fb774>] (release_nodes) from [<c02f8698>] (__device_release_driver+0x60/0xb0) > [<c02f8698>] (__device_release_driver) from [<c02f9080>] (driver_detach+0xb4/0xb8) > [<c02f9080>] (driver_detach) from [<c02f8480>] (bus_remove_driver+0x5c/0xc4) > [<c02f8480>] (bus_remove_driver) from [<c008c9b8>] (SyS_delete_module+0x148/0x1d8) > [<c008c9b8>] (SyS_delete_module) from [<c000ef80>] (ret_fast_syscall+0x0/0x48) > FIX kmalloc-128: Object at 0xed08e8d0 not freed > > Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) > Cc: Jiada Wang <jiada_wang@mentor.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Thank you for correcting this, and my apologies for introducing this bug. Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/clk/clk.c | 71 +++++++++++++++++++++++-------------------------------- > 1 file changed, 30 insertions(+), 41 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index dff0373f53c1..f71093bf83ab 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1984,9 +1984,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) { > @@ -2026,7 +2045,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) > @@ -2035,36 +2054,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); > @@ -2173,7 +2162,7 @@ EXPORT_SYMBOL_GPL(clk_unregister); > > static void devm_clk_release(struct device *dev, void *res) > { > - clk_unregister(res); > + clk_unregister(*(struct clk **)res); > } > > /** > @@ -2188,18 +2177,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; > -- Sylwester Nawrocki Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] clk: Fix double free due to devm_clk_register() @ 2014-05-14 11:26 ` Sylwester Nawrocki 0 siblings, 0 replies; 11+ messages in thread From: Sylwester Nawrocki @ 2014-05-14 11:26 UTC (permalink / raw) To: Stephen Boyd Cc: Mike Turquette, linux-kernel, linux-arm-kernel, Jiada Wang, Kyungmin Park On 19/04/14 01:29, Stephen Boyd wrote: > 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 > [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) > [<c0012240>] (show_stack) from [<c04b74dc>] (dump_stack+0x70/0xbc) > [<c04b74dc>] (dump_stack) from [<c00f6778>] (slab_err+0x74/0x84) > [<c00f6778>] (slab_err) from [<c04b6278>] (free_debug_processing+0x2cc/0x31c) > [<c04b6278>] (free_debug_processing) from [<c04b6300>] (__slab_free+0x38/0x41c) > [<c04b6300>] (__slab_free) from [<c03931bc>] (clk_unregister+0xd4/0x140) > [<c03931bc>] (clk_unregister) from [<c02fb774>] (release_nodes+0x164/0x1d8) > [<c02fb774>] (release_nodes) from [<c02f8698>] (__device_release_driver+0x60/0xb0) > [<c02f8698>] (__device_release_driver) from [<c02f9080>] (driver_detach+0xb4/0xb8) > [<c02f9080>] (driver_detach) from [<c02f8480>] (bus_remove_driver+0x5c/0xc4) > [<c02f8480>] (bus_remove_driver) from [<c008c9b8>] (SyS_delete_module+0x148/0x1d8) > [<c008c9b8>] (SyS_delete_module) from [<c000ef80>] (ret_fast_syscall+0x0/0x48) > FIX kmalloc-128: Object at 0xed08e8d0 not freed > > Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) > Cc: Jiada Wang <jiada_wang@mentor.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Thank you for correcting this, and my apologies for introducing this bug. Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/clk/clk.c | 71 +++++++++++++++++++++++-------------------------------- > 1 file changed, 30 insertions(+), 41 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index dff0373f53c1..f71093bf83ab 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1984,9 +1984,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) { > @@ -2026,7 +2045,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) > @@ -2035,36 +2054,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); > @@ -2173,7 +2162,7 @@ EXPORT_SYMBOL_GPL(clk_unregister); > > static void devm_clk_release(struct device *dev, void *res) > { > - clk_unregister(res); > + clk_unregister(*(struct clk **)res); > } > > /** > @@ -2188,18 +2177,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; > -- Sylwester Nawrocki Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] clk: Fix slab corruption in clk_unregister() 2014-04-18 23:29 ` Stephen Boyd @ 2014-04-18 23:29 ` Stephen Boyd -1 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: linux-arm-kernel When a clock is unregsitered, we iterate over the list of children and reparent them to NULL (i.e. orphan list). While iterating the list, we should use the safe iterators because the children list for this clock is changing when we reparent the children to NULL. Failure to iterate safely can lead to slab corruption like this: ============================================================================= BUG kmalloc-128 (Not tainted): Poison overwritten ----------------------------------------------------------------------------- Disabling lock debugging due to kernel taint INFO: 0xed0c4900-0xed0c4903. First byte 0x0 instead of 0x6b INFO: Allocated in clk_register+0x20/0x1bc age=297 cpu=2 pid=70 __slab_alloc.isra.39.constprop.42+0x410/0x454 kmem_cache_alloc_trace+0x200/0x24c clk_register+0x20/0x1bc devm_clk_register+0x34/0x68 0xbf0000f0 platform_drv_probe+0x18/0x48 driver_probe_device+0x94/0x360 __driver_attach+0x94/0x98 bus_for_each_dev+0x54/0x88 bus_add_driver+0xe8/0x204 driver_register+0x78/0xf4 do_one_initcall+0xc4/0x17c load_module+0x19ac/0x2294 SyS_init_module+0xa4/0x110 ret_fast_syscall+0x0/0x48 INFO: Freed in clk_unregister+0xd4/0x140 age=23 cpu=2 pid=73 __slab_free+0x38/0x41c clk_unregister+0xd4/0x140 release_nodes+0x164/0x1d8 __device_release_driver+0x60/0xb0 driver_detach+0xb4/0xb8 bus_remove_driver+0x5c/0xc4 SyS_delete_module+0x148/0x1d8 ret_fast_syscall+0x0/0x48 INFO: Slab 0xeec50b90 objects=25 used=0 fp=0xed0c5400 flags=0x4080 INFO: Object 0xed0c48c0 @offset=2240 fp=0xed0c4a00 Bytes b4 ed0c48b0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ Object ed0c48c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4900: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b ....kkkkkkkkkkkk Object ed0c4910: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4920: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4930: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk. Redzone ed0c4940: bb bb bb bb .... Padding ed0c49e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ Padding ed0c49f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ CPU: 3 PID: 75 Comm: mdev Tainted: G B 3.14.0-11033-g2054ba5ca781 #35 [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) [<c0012240>] (show_stack) from [<c04b74a0>] (dump_stack+0x70/0xbc) [<c04b74a0>] (dump_stack) from [<c00f7a78>] (check_bytes_and_report+0xbc/0x100) [<c00f7a78>] (check_bytes_and_report) from [<c00f7c48>] (check_object+0x18c/0x218) [<c00f7c48>] (check_object) from [<c00f7efc>] (__free_slab+0x104/0x144) [<c00f7efc>] (__free_slab) from [<c04b6668>] (__slab_free+0x3dc/0x41c) [<c04b6668>] (__slab_free) from [<c014c008>] (load_elf_binary+0x88/0x12b4) [<c014c008>] (load_elf_binary) from [<c0105a44>] (search_binary_handler+0x78/0x18c) [<c0105a44>] (search_binary_handler) from [<c0106fc0>] (do_execve+0x490/0x5dc) [<c0106fc0>] (do_execve) from [<c0036b8c>] (____call_usermodehelper+0x134/0x168) [<c0036b8c>] (____call_usermodehelper) from [<c000f048>] (ret_from_fork+0x14/0x2c) FIX kmalloc-128: Restoring 0xed0c4900-0xed0c4903=0x6b Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) Cc: Jiada Wang <jiada_wang@mentor.com> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index f71093bf83ab..7cf2c093cc54 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2140,9 +2140,10 @@ void clk_unregister(struct clk *clk) if (!hlist_empty(&clk->children)) { struct clk *child; + struct hlist_node *t; /* Reparent all children to the orphan list. */ - hlist_for_each_entry(child, &clk->children, child_node) + hlist_for_each_entry_safe(child, t, &clk->children, child_node) clk_set_parent(child, NULL); } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] clk: Fix slab corruption in clk_unregister() @ 2014-04-18 23:29 ` Stephen Boyd 0 siblings, 0 replies; 11+ messages in thread From: Stephen Boyd @ 2014-04-18 23:29 UTC (permalink / raw) To: Mike Turquette Cc: linux-kernel, linux-arm-kernel, Jiada Wang, Sylwester Nawrocki, Kyungmin Park When a clock is unregsitered, we iterate over the list of children and reparent them to NULL (i.e. orphan list). While iterating the list, we should use the safe iterators because the children list for this clock is changing when we reparent the children to NULL. Failure to iterate safely can lead to slab corruption like this: ============================================================================= BUG kmalloc-128 (Not tainted): Poison overwritten ----------------------------------------------------------------------------- Disabling lock debugging due to kernel taint INFO: 0xed0c4900-0xed0c4903. First byte 0x0 instead of 0x6b INFO: Allocated in clk_register+0x20/0x1bc age=297 cpu=2 pid=70 __slab_alloc.isra.39.constprop.42+0x410/0x454 kmem_cache_alloc_trace+0x200/0x24c clk_register+0x20/0x1bc devm_clk_register+0x34/0x68 0xbf0000f0 platform_drv_probe+0x18/0x48 driver_probe_device+0x94/0x360 __driver_attach+0x94/0x98 bus_for_each_dev+0x54/0x88 bus_add_driver+0xe8/0x204 driver_register+0x78/0xf4 do_one_initcall+0xc4/0x17c load_module+0x19ac/0x2294 SyS_init_module+0xa4/0x110 ret_fast_syscall+0x0/0x48 INFO: Freed in clk_unregister+0xd4/0x140 age=23 cpu=2 pid=73 __slab_free+0x38/0x41c clk_unregister+0xd4/0x140 release_nodes+0x164/0x1d8 __device_release_driver+0x60/0xb0 driver_detach+0xb4/0xb8 bus_remove_driver+0x5c/0xc4 SyS_delete_module+0x148/0x1d8 ret_fast_syscall+0x0/0x48 INFO: Slab 0xeec50b90 objects=25 used=0 fp=0xed0c5400 flags=0x4080 INFO: Object 0xed0c48c0 @offset=2240 fp=0xed0c4a00 Bytes b4 ed0c48b0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ Object ed0c48c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c48f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4900: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b ....kkkkkkkkkkkk Object ed0c4910: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4920: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk Object ed0c4930: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk. Redzone ed0c4940: bb bb bb bb .... Padding ed0c49e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ Padding ed0c49f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ CPU: 3 PID: 75 Comm: mdev Tainted: G B 3.14.0-11033-g2054ba5ca781 #35 [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) [<c0012240>] (show_stack) from [<c04b74a0>] (dump_stack+0x70/0xbc) [<c04b74a0>] (dump_stack) from [<c00f7a78>] (check_bytes_and_report+0xbc/0x100) [<c00f7a78>] (check_bytes_and_report) from [<c00f7c48>] (check_object+0x18c/0x218) [<c00f7c48>] (check_object) from [<c00f7efc>] (__free_slab+0x104/0x144) [<c00f7efc>] (__free_slab) from [<c04b6668>] (__slab_free+0x3dc/0x41c) [<c04b6668>] (__slab_free) from [<c014c008>] (load_elf_binary+0x88/0x12b4) [<c014c008>] (load_elf_binary) from [<c0105a44>] (search_binary_handler+0x78/0x18c) [<c0105a44>] (search_binary_handler) from [<c0106fc0>] (do_execve+0x490/0x5dc) [<c0106fc0>] (do_execve) from [<c0036b8c>] (____call_usermodehelper+0x134/0x168) [<c0036b8c>] (____call_usermodehelper) from [<c000f048>] (ret_from_fork+0x14/0x2c) FIX kmalloc-128: Restoring 0xed0c4900-0xed0c4903=0x6b Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) Cc: Jiada Wang <jiada_wang@mentor.com> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clk/clk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index f71093bf83ab..7cf2c093cc54 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2140,9 +2140,10 @@ void clk_unregister(struct clk *clk) if (!hlist_empty(&clk->children)) { struct clk *child; + struct hlist_node *t; /* Reparent all children to the orphan list. */ - hlist_for_each_entry(child, &clk->children, child_node) + hlist_for_each_entry_safe(child, t, &clk->children, child_node) clk_set_parent(child, NULL); } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] clk: Fix slab corruption in clk_unregister() 2014-04-18 23:29 ` Stephen Boyd @ 2014-05-14 11:27 ` Sylwester Nawrocki -1 siblings, 0 replies; 11+ messages in thread From: Sylwester Nawrocki @ 2014-05-14 11:27 UTC (permalink / raw) To: linux-arm-kernel On 19/04/14 01:29, Stephen Boyd wrote: > When a clock is unregsitered, we iterate over the list of > children and reparent them to NULL (i.e. orphan list). While > iterating the list, we should use the safe iterators because the > children list for this clock is changing when we reparent the > children to NULL. Failure to iterate safely can lead to slab > corruption like this: > > ============================================================================= > BUG kmalloc-128 (Not tainted): Poison overwritten > ----------------------------------------------------------------------------- > > Disabling lock debugging due to kernel taint > INFO: 0xed0c4900-0xed0c4903. First byte 0x0 instead of 0x6b > INFO: Allocated in clk_register+0x20/0x1bc age=297 cpu=2 pid=70 > __slab_alloc.isra.39.constprop.42+0x410/0x454 > kmem_cache_alloc_trace+0x200/0x24c > clk_register+0x20/0x1bc > devm_clk_register+0x34/0x68 > 0xbf0000f0 > platform_drv_probe+0x18/0x48 > driver_probe_device+0x94/0x360 > __driver_attach+0x94/0x98 > bus_for_each_dev+0x54/0x88 > bus_add_driver+0xe8/0x204 > driver_register+0x78/0xf4 > do_one_initcall+0xc4/0x17c > load_module+0x19ac/0x2294 > SyS_init_module+0xa4/0x110 > ret_fast_syscall+0x0/0x48 > INFO: Freed in clk_unregister+0xd4/0x140 age=23 cpu=2 pid=73 > __slab_free+0x38/0x41c > clk_unregister+0xd4/0x140 > release_nodes+0x164/0x1d8 > __device_release_driver+0x60/0xb0 > driver_detach+0xb4/0xb8 > bus_remove_driver+0x5c/0xc4 > SyS_delete_module+0x148/0x1d8 > ret_fast_syscall+0x0/0x48 > INFO: Slab 0xeec50b90 objects=25 used=0 fp=0xed0c5400 flags=0x4080 > INFO: Object 0xed0c48c0 @offset=2240 fp=0xed0c4a00 > > Bytes b4 ed0c48b0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ > Object ed0c48c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4900: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b ....kkkkkkkkkkkk > Object ed0c4910: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4920: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4930: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk. > Redzone ed0c4940: bb bb bb bb .... > Padding ed0c49e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ > Padding ed0c49f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ > CPU: 3 PID: 75 Comm: mdev Tainted: G B 3.14.0-11033-g2054ba5ca781 #35 > [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) > [<c0012240>] (show_stack) from [<c04b74a0>] (dump_stack+0x70/0xbc) > [<c04b74a0>] (dump_stack) from [<c00f7a78>] (check_bytes_and_report+0xbc/0x100) > [<c00f7a78>] (check_bytes_and_report) from [<c00f7c48>] (check_object+0x18c/0x218) > [<c00f7c48>] (check_object) from [<c00f7efc>] (__free_slab+0x104/0x144) > [<c00f7efc>] (__free_slab) from [<c04b6668>] (__slab_free+0x3dc/0x41c) > [<c04b6668>] (__slab_free) from [<c014c008>] (load_elf_binary+0x88/0x12b4) > [<c014c008>] (load_elf_binary) from [<c0105a44>] (search_binary_handler+0x78/0x18c) > [<c0105a44>] (search_binary_handler) from [<c0106fc0>] (do_execve+0x490/0x5dc) > [<c0106fc0>] (do_execve) from [<c0036b8c>] (____call_usermodehelper+0x134/0x168) > [<c0036b8c>] (____call_usermodehelper) from [<c000f048>] (ret_from_fork+0x14/0x2c) > FIX kmalloc-128: Restoring 0xed0c4900-0xed0c4903=0x6b > > Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) > Cc: Jiada Wang <jiada_wang@mentor.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/clk/clk.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index f71093bf83ab..7cf2c093cc54 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -2140,9 +2140,10 @@ void clk_unregister(struct clk *clk) > > if (!hlist_empty(&clk->children)) { > struct clk *child; > + struct hlist_node *t; > > /* Reparent all children to the orphan list. */ > - hlist_for_each_entry(child, &clk->children, child_node) > + hlist_for_each_entry_safe(child, t, &clk->children, child_node) > clk_set_parent(child, NULL); > } > > -- Sylwester Nawrocki Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] clk: Fix slab corruption in clk_unregister() @ 2014-05-14 11:27 ` Sylwester Nawrocki 0 siblings, 0 replies; 11+ messages in thread From: Sylwester Nawrocki @ 2014-05-14 11:27 UTC (permalink / raw) To: Stephen Boyd Cc: Mike Turquette, linux-kernel, linux-arm-kernel, Jiada Wang, Kyungmin Park On 19/04/14 01:29, Stephen Boyd wrote: > When a clock is unregsitered, we iterate over the list of > children and reparent them to NULL (i.e. orphan list). While > iterating the list, we should use the safe iterators because the > children list for this clock is changing when we reparent the > children to NULL. Failure to iterate safely can lead to slab > corruption like this: > > ============================================================================= > BUG kmalloc-128 (Not tainted): Poison overwritten > ----------------------------------------------------------------------------- > > Disabling lock debugging due to kernel taint > INFO: 0xed0c4900-0xed0c4903. First byte 0x0 instead of 0x6b > INFO: Allocated in clk_register+0x20/0x1bc age=297 cpu=2 pid=70 > __slab_alloc.isra.39.constprop.42+0x410/0x454 > kmem_cache_alloc_trace+0x200/0x24c > clk_register+0x20/0x1bc > devm_clk_register+0x34/0x68 > 0xbf0000f0 > platform_drv_probe+0x18/0x48 > driver_probe_device+0x94/0x360 > __driver_attach+0x94/0x98 > bus_for_each_dev+0x54/0x88 > bus_add_driver+0xe8/0x204 > driver_register+0x78/0xf4 > do_one_initcall+0xc4/0x17c > load_module+0x19ac/0x2294 > SyS_init_module+0xa4/0x110 > ret_fast_syscall+0x0/0x48 > INFO: Freed in clk_unregister+0xd4/0x140 age=23 cpu=2 pid=73 > __slab_free+0x38/0x41c > clk_unregister+0xd4/0x140 > release_nodes+0x164/0x1d8 > __device_release_driver+0x60/0xb0 > driver_detach+0xb4/0xb8 > bus_remove_driver+0x5c/0xc4 > SyS_delete_module+0x148/0x1d8 > ret_fast_syscall+0x0/0x48 > INFO: Slab 0xeec50b90 objects=25 used=0 fp=0xed0c5400 flags=0x4080 > INFO: Object 0xed0c48c0 @offset=2240 fp=0xed0c4a00 > > Bytes b4 ed0c48b0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ > Object ed0c48c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c48f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4900: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b ....kkkkkkkkkkkk > Object ed0c4910: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4920: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk > Object ed0c4930: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk. > Redzone ed0c4940: bb bb bb bb .... > Padding ed0c49e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ > Padding ed0c49f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ > CPU: 3 PID: 75 Comm: mdev Tainted: G B 3.14.0-11033-g2054ba5ca781 #35 > [<c0014be0>] (unwind_backtrace) from [<c0012240>] (show_stack+0x10/0x14) > [<c0012240>] (show_stack) from [<c04b74a0>] (dump_stack+0x70/0xbc) > [<c04b74a0>] (dump_stack) from [<c00f7a78>] (check_bytes_and_report+0xbc/0x100) > [<c00f7a78>] (check_bytes_and_report) from [<c00f7c48>] (check_object+0x18c/0x218) > [<c00f7c48>] (check_object) from [<c00f7efc>] (__free_slab+0x104/0x144) > [<c00f7efc>] (__free_slab) from [<c04b6668>] (__slab_free+0x3dc/0x41c) > [<c04b6668>] (__slab_free) from [<c014c008>] (load_elf_binary+0x88/0x12b4) > [<c014c008>] (load_elf_binary) from [<c0105a44>] (search_binary_handler+0x78/0x18c) > [<c0105a44>] (search_binary_handler) from [<c0106fc0>] (do_execve+0x490/0x5dc) > [<c0106fc0>] (do_execve) from [<c0036b8c>] (____call_usermodehelper+0x134/0x168) > [<c0036b8c>] (____call_usermodehelper) from [<c000f048>] (ret_from_fork+0x14/0x2c) > FIX kmalloc-128: Restoring 0xed0c4900-0xed0c4903=0x6b > > Fixes: fcb0ee6a3d33 (clk: Implement clk_unregister) > Cc: Jiada Wang <jiada_wang@mentor.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/clk/clk.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index f71093bf83ab..7cf2c093cc54 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -2140,9 +2140,10 @@ void clk_unregister(struct clk *clk) > > if (!hlist_empty(&clk->children)) { > struct clk *child; > + struct hlist_node *t; > > /* Reparent all children to the orphan list. */ > - hlist_for_each_entry(child, &clk->children, child_node) > + hlist_for_each_entry_safe(child, t, &clk->children, child_node) > clk_set_parent(child, NULL); > } > > -- Sylwester Nawrocki Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/2] Clock unregistration fixes 2014-04-18 23:29 ` Stephen Boyd ` (2 preceding siblings ...) (?) @ 2014-04-29 7:24 ` Mike Turquette -1 siblings, 0 replies; 11+ messages in thread From: Mike Turquette @ 2014-04-29 7:24 UTC (permalink / raw) To: linux-arm-kernel Quoting Stephen Boyd (2014-04-18 16:29:41) > Two fixes for fallout from commit fcb0ee6a3d33 (clk: Implement clk_unregister). > > Stephen Boyd (2): > clk: Fix double free due to devm_clk_register() > clk: Fix slab corruption in clk_unregister() Thanks for the fixes. I've Cc'd them to stable as well for 3.14. Regards, Mike > > drivers/clk/clk.c | 74 ++++++++++++++++++++++++------------------------------- > 1 file changed, 32 insertions(+), 42 deletions(-) > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > hosted by The Linux Foundation > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-05-14 11:27 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-18 23:29 [PATCH 0/2] Clock unregistration fixes Stephen Boyd 2014-04-18 23:29 ` Stephen Boyd 2014-04-18 23:29 ` [PATCH 1/2] clk: Fix double free due to devm_clk_register() Stephen Boyd 2014-04-18 23:29 ` Stephen Boyd 2014-05-14 11:26 ` Sylwester Nawrocki 2014-05-14 11:26 ` Sylwester Nawrocki 2014-04-18 23:29 ` [PATCH 2/2] clk: Fix slab corruption in clk_unregister() Stephen Boyd 2014-04-18 23:29 ` Stephen Boyd 2014-05-14 11:27 ` Sylwester Nawrocki 2014-05-14 11:27 ` Sylwester Nawrocki 2014-04-29 7:24 ` [PATCH 0/2] Clock unregistration fixes Mike Turquette
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.