[B.A.T.M.A.N.] [PATCH net] batman-adv: Fix memory leak on tt add with invalid vlan

[B.A.T.M.A.N.] [PATCH net] batman-adv: Fix memory leak on tt add with invalid vlan

[Sven Eckelmann]

The object tt_local is allocated with kmalloc and not initialized when the
function batadv_tt_local_add checks for the vlan. But this function can
only cleanup the object when the (not yet initialized) reference counter of
the object is 1. This is unlikely and thus the object would leak when the
vlan could not be found.

Instead the uninitialized object tt_local has to be freed manually and the
pointer has to set to NULL to avoid calling the function which would try to
decrement the reference counter of the not existing object.

CID: 1316518
Fixes: 354136bcc3c4 ("batman-adv: fix kernel crash due to missing NULL checks")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Hi,

sorry for sending the patch directly to you. But Marek told me that neither
he or Antonio have currently time to prepare/send a patch for this problem.
Instead he told me that it would be ok to send the patch directly to you.

The problematic patch is currently in net.git and was accepted a week ago
by you [1]. I was also told that this patch has also been queued for
different stable trees.

Kind regards,
	Sven

[1] http://www.spinics.net/lists/netdev/msg338630.html
---
 net/batman-adv/translation-table.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 75bc678ea116..4228b10c47ea 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -598,8 +598,11 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const u8 *addr,
 	/* increase the refcounter of the related vlan */
 	vlan = batadv_softif_vlan_get(bat_priv, vid);
 	if (WARN(!vlan, "adding TT local entry %pM to non-existent VLAN %d",
-		 addr, BATADV_PRINT_VID(vid)))
+		 addr, BATADV_PRINT_VID(vid))) {
+		kfree(tt_local);
+		tt_local = NULL;
 		goto out;
+	}
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Creating new local tt entry: %pM (vid: %d, ttvn: %d)\n",
-- 
2.5.0

Re: [B.A.T.M.A.N.] [PATCH net] batman-adv: Fix memory leak on tt add with invalid vlan

[David Miller]

From: Sven Eckelmann <sven@narfation.org>
Date: Tue, 18 Aug 2015 13:37:01 +0200

> The object tt_local is allocated with kmalloc and not initialized when the
> function batadv_tt_local_add checks for the vlan. But this function can
> only cleanup the object when the (not yet initialized) reference counter of
> the object is 1. This is unlikely and thus the object would leak when the
> vlan could not be found.
> 
> Instead the uninitialized object tt_local has to be freed manually and the
> pointer has to set to NULL to avoid calling the function which would try to
> decrement the reference counter of the not existing object.
> 
> CID: 1316518
> Fixes: 354136bcc3c4 ("batman-adv: fix kernel crash due to missing NULL checks")
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

Applied, thanks.

Re: [B.A.T.M.A.N.] [PATCH net] batman-adv: Fix memory leak on tt add with invalid vlan

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On Tuesday, August 18, 2015 13:37:01 Sven Eckelmann wrote:
> The object tt_local is allocated with kmalloc and not initialized when the
> function batadv_tt_local_add checks for the vlan. But this function can
> only cleanup the object when the (not yet initialized) reference counter of
> the object is 1. This is unlikely and thus the object would leak when the
> vlan could not be found.
> 
> Instead the uninitialized object tt_local has to be freed manually and the
> pointer has to set to NULL to avoid calling the function which would try to
> decrement the reference counter of the not existing object.

Applied in revision dba67bc.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]