Re: [PATCH v4] clocksource/timer-imx-gpt: Prevent resource leake in error path.

From: Thomas Gleixner
Date: Wed Sep 28 2016 - 15:13:11 EST

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

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.