netdev - RE: [PATCH 2/4] batman-adv: fix kernel crash due to missing NULL checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CB737B7@AcuExch.aculab.com>
Date:	Wed, 5 Aug 2015 13:15:14 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Antonio Quartulli' <antonio@...hcoding.com>,
	"davem@...emloft.net" <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"b.a.t.m.a.n@...ts.open-mesh.org" <b.a.t.m.a.n@...ts.open-mesh.org>,
	"Marek Lindner" <mareklindner@...mailbox.ch>
Subject: RE: [PATCH 2/4] batman-adv: fix kernel crash due to missing NULL
 checks

From: Of Antonio Quartulli
> Sent: 05 August 2015 13:52
> From: Marek Lindner <mareklindner@...mailbox.ch>
> 
> batadv_softif_vlan_get() may return NULL which has to be verified
> by the caller.
> 
...
> diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
> index c002961..a2fc843 100644
> --- a/net/batman-adv/soft-interface.c
> +++ b/net/batman-adv/soft-interface.c
> @@ -479,6 +479,9 @@ out:
>   */
>  void batadv_softif_vlan_free_ref(struct batadv_softif_vlan *vlan)
>  {
> +	if (!vlan)
> +		return;
> +

This bit doesn't look necessary.
You've added checks to some callers, the others probably don't need the check.

> @@ -1066,6 +1069,9 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv,
> 
>  	/* decrease the reference held for this vlan */
>  	vlan = batadv_softif_vlan_get(bat_priv, vid);
> +	if (!vlan)
> +		goto out;
> +
>  	batadv_softif_vlan_free_ref(vlan);
>  	batadv_softif_vlan_free_ref(vlan);

That code is ringing alarm bells.
If you expect to have a reference count the object better exist.
If you can remove a reference count from a 'random' object then
you can break the reference counting of objects.

So is this test just hiding anoter bug somewhere??

	David

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html