Messages in this thread | | | Date | Wed, 28 Sep 2016 15:10:27 -0400 (EDT) | From | Thomas Gleixner <> | Subject | Re: [PATCH v4] clocksource/timer-imx-gpt: Prevent resource leake in error path. |
| |
On Wed, 28 Sep 2016, Arvind Yadav wrote:
> -Free previously allocated memory. > -Unmap I/O memory from kernel address space.
Ok. So we have proper looking patch with a proper looking changelog now. Though
> @@ -507,11 +511,17 @@ static int __init mxc_timer_init_dt(struct device_node *np, enum imx_gpt_type t > > ret = _mxc_timer_init(imxtm); > if (ret) > - return ret; > + goto error_iounmap;
Have you actually thought about the implications of this?
Assume the last function call() which gets invoked via _mxc_timer_init() fails and then figure out what the iounmap() and the kfree() are going to cause.
Hint: You just turned a resource leak into a fatal crash.
Just slapping unmap/kfree blindly at everything which can return a failure is not a good idea if you do not check what the functions actually do and what the error pathes there are.
While I agree that the error handling in this and in other drivers is lousy, fixing it just mechanicaly and thereby introducing harder to debug wreckage is certainly not the right approach.
Thanks,
tglx
| |