| Message ID | 20191026045331.1097-1-navid.emamdoost@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | wimax: i2400: Fix memory leak in i2400m_op_rfkill_sw_toggle | expand |
> Move kfree(cmd) before return to be reached by all execution paths. I suggest to reconsider this change suggestion once more. > Fixes: 2507e6ab7a9a ("wimax: i2400: fix memory leak") I find it interesting that you would like to fix your commit from 2019-09-10. https://lore.kernel.org/patchwork/patch/1126399/ Will it be helpful to refer also to the commit 024f7f31ed15c471f80408d8b5045497e27e1135 ("i2400m: Generic probe/disconnect, reset and message passing" from 2009-01-07)? > +++ b/drivers/net/wimax/i2400m/op-rfkill.c > @@ -127,12 +127,12 @@ int i2400m_op_rfkill_sw_toggle(struct wimax_dev *wimax_dev, > "%d\n", result); > result = 0; > error_cmd: > - kfree(cmd); > kfree_skb(ack_skb); > error_msg_to_dev: > error_alloc: > d_fnend(4, dev, "(wimax_dev %p state %d) = %d\n", > wimax_dev, state, result); > + kfree(cmd); > return result; > } I would prefer to improve the exception handling like the following. (Would you like to avoid passing a null pointer at the end?) -error_cmd: +free_skb: - kfree(cmd); kfree_skb(ack_skb); -error_msg_to_dev: +free_cmd: + kfree(cmd); -error_alloc: +exit: How do you think about this update variant? Regards, Markus
From: Navid Emamdoost <navid.emamdoost@gmail.com> Date: Fri, 25 Oct 2019 23:53:30 -0500 > In the implementation of i2400m_op_rfkill_sw_toggle() the allocated > buffer for cmd should be released before returning. The > documentation for i2400m_msg_to_dev() says when it returns the buffer > can be reused. Meaning cmd should be released in either case. Move > kfree(cmd) before return to be reached by all execution paths. > > Fixes: 2507e6ab7a9a ("wimax: i2400: fix memory leak") > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> Applied.
diff --git a/drivers/net/wimax/i2400m/op-rfkill.c b/drivers/net/wimax/i2400m/op-rfkill.c index 8efb493ceec2..5c79f052cad2 100644 --- a/drivers/net/wimax/i2400m/op-rfkill.c +++ b/drivers/net/wimax/i2400m/op-rfkill.c @@ -127,12 +127,12 @@ int i2400m_op_rfkill_sw_toggle(struct wimax_dev *wimax_dev, "%d\n", result); result = 0; error_cmd: - kfree(cmd); kfree_skb(ack_skb); error_msg_to_dev: error_alloc: d_fnend(4, dev, "(wimax_dev %p state %d) = %d\n", wimax_dev, state, result); + kfree(cmd); return result; }
In the implementation of i2400m_op_rfkill_sw_toggle() the allocated buffer for cmd should be released before returning. The documentation for i2400m_msg_to_dev() says when it returns the buffer can be reused. Meaning cmd should be released in either case. Move kfree(cmd) before return to be reached by all execution paths. Fixes: 2507e6ab7a9a ("wimax: i2400: fix memory leak") Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> --- drivers/net/wimax/i2400m/op-rfkill.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)