2020-04-15 22:35:05

by Xiyu Yang

[permalink] [raw]
Subject: [PATCH] batman-adv: Fix refcnt leak in batadv_show_throughput_override

batadv_show_throughput_override() invokes batadv_hardif_get_by_netdev(),
which gets a batadv_hard_iface object from net_dev with increased refcnt
and its reference is assigned to a local pointer 'hard_iface'.

When batadv_show_throughput_override() returns, "hard_iface" becomes
invalid, so the refcount should be decreased to keep refcount balanced.

The issue happens in the normal path of
batadv_show_throughput_override(), which forgets to decrease the refcnt
increased by batadv_hardif_get_by_netdev() before the function returns,
causing a refcnt leak.

Fix this issue by calling batadv_hardif_put() before the
batadv_show_throughput_override() returns in the normal path.

Signed-off-by: Xiyu Yang <[email protected]>
Signed-off-by: Xin Tan <[email protected]>
---
net/batman-adv/sysfs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 97736696d042..0f962dcd239e 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -1190,6 +1190,7 @@ static ssize_t batadv_show_throughput_override(struct kobject *kobj,

tp_override = atomic_read(&hard_iface->bat_v.throughput_override);

+ batadv_hardif_put(hard_iface);
return sprintf(buff, "%u.%u MBit\n", tp_override / 10,
tp_override % 10);
}
--
2.7.4


2020-04-15 22:45:47

by Sven Eckelmann

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Fix refcnt leak in batadv_show_throughput_override

On Wednesday, 15 April 2020 10:31:50 CEST Xiyu Yang wrote:
[...]
> Fix this issue by calling batadv_hardif_put() before the
[...]

Thanks, fixes for batadv_store_throughput_override [1] and
batadv_show_throughput_override [2] were applied. I've also added the missing
Fixes: line to both patches.

May I ask whether you are still a user of the deprecated sysfs interface or
did you find this in an automated fashion?

Thanks,
Sven

[1] https://git.open-mesh.org/linux-merge.git/commit/cd339d8b14cd895d8333d94d832b05f67f00eefc
[2] https://git.open-mesh.org/linux-merge.git/commit/3d3e548f74fe51aee9a3c9e297518a2655dbc642


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part.

2020-04-15 22:58:36

by Xiyu Yang

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Fix refcnt leak in batadv_show_throughput_override

On Wed, Apr 15, 2020 at 11:04:02AM +0200, Sven Eckelmann wrote:
> On Wednesday, 15 April 2020 10:31:50 CEST Xiyu Yang wrote:
> [...]
> > Fix this issue by calling batadv_hardif_put() before the
> [...]
>
> Thanks, fixes for batadv_store_throughput_override [1] and
> batadv_show_throughput_override [2] were applied. I've also added the missing
> Fixes: line to both patches.
>
> May I ask whether you are still a user of the deprecated sysfs interface or
> did you find this in an automated fashion?
>
> Thanks,
> Sven
>
> [1] https://git.open-mesh.org/linux-merge.git/commit/cd339d8b14cd895d8333d94d832b05f67f00eefc
> [2] https://git.open-mesh.org/linux-merge.git/commit/3d3e548f74fe51aee9a3c9e297518a2655dbc642

Thanks for your confirmation! We are looking for some automated ways to find this kind of bug.