* [B.A.T.M.A.N.] [PATCH] RFC: batman-adv: Fix null pointer deref. when adding hard-if as soft-if
2011-03-03 19:09 [B.A.T.M.A.N.] Null pointer dereference, ticket #146 Linus Lüssing
@ 2011-03-03 19:09 ` Linus Lüssing
2011-03-03 20:22 ` [B.A.T.M.A.N.] Null pointer dereference, ticket #146 Sven Eckelmann
2011-03-03 20:39 ` [B.A.T.M.A.N.] [PATCH] batman-adv: Fix null pointer deref. when adding hard-if as soft-if Sven Eckelmann
2 siblings, 0 replies; 9+ messages in thread
From: Linus Lüssing @ 2011-03-03 19:09 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Linus Lüssing
When we are trying to create a batman soft-interface which already
exists as a common hard interface, batman than wrongly assumes that this
hard interface is a fully initialized soft interface. This leads to a
null pointer dereference on the first try of accessing for instance a
non-intialized orig_hash.
For every hard interface, there is no initialized orig_hash, therefore
this commit uses this criteria to abort creating a soft interface with
an already existing name.
---
hard-interface.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 95a35b6..53d6fce 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -282,6 +282,7 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
{
struct bat_priv *bat_priv;
struct batman_packet *batman_packet;
+ int ret;
if (hard_iface->if_status != IF_NOT_IN_USE)
goto out;
@@ -294,20 +295,32 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
if (!hard_iface->soft_iface) {
hard_iface->soft_iface = softif_create(iface_name);
- if (!hard_iface->soft_iface)
+ if (!hard_iface->soft_iface) {
+ ret = -ENOMEM;
goto err;
+ }
/* dev_get_by_name() increases the reference counter for us */
dev_hold(hard_iface->soft_iface);
}
bat_priv = netdev_priv(hard_iface->soft_iface);
+
+ if (!bat_priv->orig_hash) {
+ bat_err(hard_iface->soft_iface,
+ "Can't create soft interface %s: "
+ "already exists as non soft interface\n",
+ hard_iface->soft_iface->name);
+ ret = -EINVAL;
+ goto err;
+ }
hard_iface->packet_len = BAT_PACKET_LEN;
hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
if (!hard_iface->packet_buff) {
bat_err(hard_iface->soft_iface, "Can't add interface packet "
"(%s): out of memory\n", hard_iface->net_dev->name);
+ ret = -ENOMEM;
goto err;
}
@@ -370,7 +383,7 @@ out:
err:
hardif_free_ref(hard_iface);
- return -ENOMEM;
+ return ret;
}
void hardif_disable_interface(struct hard_iface *hard_iface)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [B.A.T.M.A.N.] Null pointer dereference, ticket #146
2011-03-03 19:09 [B.A.T.M.A.N.] Null pointer dereference, ticket #146 Linus Lüssing
2011-03-03 19:09 ` [B.A.T.M.A.N.] [PATCH] RFC: batman-adv: Fix null pointer deref. when adding hard-if as soft-if Linus Lüssing
@ 2011-03-03 20:22 ` Sven Eckelmann
2011-03-03 20:39 ` [B.A.T.M.A.N.] [PATCH] batman-adv: Fix null pointer deref. when adding hard-if as soft-if Sven Eckelmann
2 siblings, 0 replies; 9+ messages in thread
From: Sven Eckelmann @ 2011-03-03 20:22 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Linus Lüssing
[-- Attachment #1: Type: Text/Plain, Size: 987 bytes --]
Linus Lüssing wrote:
> Hi everyone,
>
> By accident I've typed in a mesh interface name for batman-adv which
> already existed as a real interface. This produces a null pointer
> dereference in orig_hash_add_if():
> http://www.open-mesh.org/ticket/146
>
> The attached patch shall illustrate the problem, but I'm not quite
> satisfied with it. Although it seems to "fix" the problem and gets
> rid of the call trace, it is probably still very racy. Does anyone
> have an idea for a more sane but equally easy check to fix the issue?
> Or is the only sane solution to hold an rcu-lock and compare the
> hard_iface->soft_iface in hardif_enable_interface() with every
> hard-iface->net_dev from the hardif_list?
>
> Cheers, Linus
Oh, it doesn't fix anything - it just works by accident. :)
You are just happy that bat_priv->orig_hash is still memory that is accessible
by us and is zero.
Let me suggest another patch (may take a while).
Best regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [B.A.T.M.A.N.] [PATCH] batman-adv: Fix null pointer deref. when adding hard-if as soft-if
2011-03-03 19:09 [B.A.T.M.A.N.] Null pointer dereference, ticket #146 Linus Lüssing
2011-03-03 19:09 ` [B.A.T.M.A.N.] [PATCH] RFC: batman-adv: Fix null pointer deref. when adding hard-if as soft-if Linus Lüssing
2011-03-03 20:22 ` [B.A.T.M.A.N.] Null pointer dereference, ticket #146 Sven Eckelmann
@ 2011-03-03 20:39 ` Sven Eckelmann
2011-03-03 21:08 ` [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device Sven Eckelmann
2 siblings, 1 reply; 9+ messages in thread
From: Sven Eckelmann @ 2011-03-03 20:39 UTC (permalink / raw)
To: b.a.t.m.a.n
When we are trying to create a batman soft-interface which already
exists as a common hard interface, batman wrongly assumes that this
hard interface is a fully initialized soft interface. This leads to a
null pointer dereference on the first try of accessing for instance a
non-intialized orig_hash.
Reported-by: Linus Lüssing <linus.luessing@ascom.ch>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
batman-adv/hard-interface.c | 37 +++++++++++++++++++++++--------------
batman-adv/soft-interface.c | 13 +++++++++++++
batman-adv/soft-interface.h | 1 +
3 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 95a35b6..947e647 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -71,21 +71,14 @@ static int is_valid_iface(struct net_device *net_dev)
{
if (net_dev->flags & IFF_LOOPBACK)
return 0;
-
if (net_dev->type != ARPHRD_ETHER)
return 0;
-
if (net_dev->addr_len != ETH_ALEN)
return 0;
/* no batman over batman */
-#ifdef HAVE_NET_DEVICE_OPS
- if (net_dev->netdev_ops->ndo_start_xmit == interface_tx)
+ if (softif_is_valid(net_dev))
return 0;
-#else
- if (net_dev->hard_start_xmit == interface_tx)
- return 0;
-#endif
/* Device is being bridged */
/* if (net_dev->priv_flags & IFF_BRIDGE_PORT)
@@ -282,6 +275,8 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
{
struct bat_priv *bat_priv;
struct batman_packet *batman_packet;
+ int ret;
+ struct net_device *soft_iface;
if (hard_iface->if_status != IF_NOT_IN_USE)
goto out;
@@ -289,18 +284,31 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
if (!atomic_inc_not_zero(&hard_iface->refcount))
goto out;
- hard_iface->soft_iface = dev_get_by_name(&init_net, iface_name);
+ soft_iface = dev_get_by_name(&init_net, iface_name);
- if (!hard_iface->soft_iface) {
- hard_iface->soft_iface = softif_create(iface_name);
+ if (!soft_iface) {
+ soft_iface = softif_create(iface_name);
- if (!hard_iface->soft_iface)
+ if (!soft_iface) {
+ ret = -ENOMEM;
goto err;
+ }
/* dev_get_by_name() increases the reference counter for us */
- dev_hold(hard_iface->soft_iface);
+ dev_hold(soft_iface);
}
+ if (!softif_is_valid(soft_iface)) {
+ pr_err("Can't create soft interface %s: "
+ "already exists as non soft interface\n",
+ soft_iface->name);
+ dev_put(soft_iface);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ hard_iface->soft_iface = soft_iface;
+
bat_priv = netdev_priv(hard_iface->soft_iface);
hard_iface->packet_len = BAT_PACKET_LEN;
hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
@@ -308,6 +316,7 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
if (!hard_iface->packet_buff) {
bat_err(hard_iface->soft_iface, "Can't add interface packet "
"(%s): out of memory\n", hard_iface->net_dev->name);
+ ret = -ENOMEM;
goto err;
}
@@ -370,7 +379,7 @@ out:
err:
hardif_free_ref(hard_iface);
- return -ENOMEM;
+ return ret;
}
void hardif_disable_interface(struct hard_iface *hard_iface)
diff --git a/batman-adv/soft-interface.c b/batman-adv/soft-interface.c
index 6b514ec..9ed2614 100644
--- a/batman-adv/soft-interface.c
+++ b/batman-adv/soft-interface.c
@@ -622,6 +622,19 @@ void softif_destroy(struct net_device *soft_iface)
unregister_netdevice(soft_iface);
}
+int softif_is_valid(struct net_device *net_dev)
+{
+#ifdef HAVE_NET_DEVICE_OPS
+ if (net_dev->netdev_ops->ndo_start_xmit == interface_tx)
+ return 1;
+#else
+ if (net_dev->hard_start_xmit == interface_tx)
+ return 1;
+#endif
+
+ return 0;
+}
+
/* ethtool */
static int bat_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
diff --git a/batman-adv/soft-interface.h b/batman-adv/soft-interface.h
index 80a3607..4789b6f 100644
--- a/batman-adv/soft-interface.h
+++ b/batman-adv/soft-interface.h
@@ -31,5 +31,6 @@ void interface_rx(struct net_device *soft_iface,
int hdr_size);
struct net_device *softif_create(char *name);
void softif_destroy(struct net_device *soft_iface);
+int softif_is_valid(struct net_device *net_dev);
#endif /* _NET_BATMAN_ADV_SOFT_INTERFACE_H_ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device
2011-03-03 20:39 ` [B.A.T.M.A.N.] [PATCH] batman-adv: Fix null pointer deref. when adding hard-if as soft-if Sven Eckelmann
@ 2011-03-03 21:08 ` Sven Eckelmann
2011-03-04 20:55 ` Marek Lindner
2011-03-04 21:34 ` Marek Lindner
0 siblings, 2 replies; 9+ messages in thread
From: Sven Eckelmann @ 2011-03-03 21:08 UTC (permalink / raw)
To: b.a.t.m.a.n
When trying to associate a net_device with another net_device which
already exists, batman-adv assumes that this interface is a fully
initialized batman mesh interface without checking it. The behaviour
when accessing data behind netdev_priv of a random net_device is
undefined and potentially dangerous.
Reported-by: Linus Lüssing <linus.luessing@ascom.ch>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
* Just smaller cleanups
* No functionality changes
batman-adv/hard-interface.c | 34 ++++++++++++++++++++++------------
batman-adv/soft-interface.c | 13 +++++++++++++
batman-adv/soft-interface.h | 1 +
3 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 95a35b6..a5d88e5 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -79,13 +79,8 @@ static int is_valid_iface(struct net_device *net_dev)
return 0;
/* no batman over batman */
-#ifdef HAVE_NET_DEVICE_OPS
- if (net_dev->netdev_ops->ndo_start_xmit == interface_tx)
+ if (softif_is_valid(net_dev))
return 0;
-#else
- if (net_dev->hard_start_xmit == interface_tx)
- return 0;
-#endif
/* Device is being bridged */
/* if (net_dev->priv_flags & IFF_BRIDGE_PORT)
@@ -282,6 +277,8 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
{
struct bat_priv *bat_priv;
struct batman_packet *batman_packet;
+ struct net_device *soft_iface;
+ int ret;
if (hard_iface->if_status != IF_NOT_IN_USE)
goto out;
@@ -289,18 +286,30 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
if (!atomic_inc_not_zero(&hard_iface->refcount))
goto out;
- hard_iface->soft_iface = dev_get_by_name(&init_net, iface_name);
+ soft_iface = dev_get_by_name(&init_net, iface_name);
- if (!hard_iface->soft_iface) {
- hard_iface->soft_iface = softif_create(iface_name);
+ if (!soft_iface) {
+ soft_iface = softif_create(iface_name);
- if (!hard_iface->soft_iface)
+ if (!soft_iface) {
+ ret = -ENOMEM;
goto err;
+ }
/* dev_get_by_name() increases the reference counter for us */
- dev_hold(hard_iface->soft_iface);
+ dev_hold(soft_iface);
}
+ if (!softif_is_valid(soft_iface)) {
+ pr_err("Can't create batman mesh interface interface %s: "
+ "already exists as regular interface\n",
+ soft_iface->name);
+ dev_put(soft_iface);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ hard_iface->soft_iface = soft_iface;
bat_priv = netdev_priv(hard_iface->soft_iface);
hard_iface->packet_len = BAT_PACKET_LEN;
hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
@@ -308,6 +317,7 @@ int hardif_enable_interface(struct hard_iface *hard_iface, char *iface_name)
if (!hard_iface->packet_buff) {
bat_err(hard_iface->soft_iface, "Can't add interface packet "
"(%s): out of memory\n", hard_iface->net_dev->name);
+ ret = -ENOMEM;
goto err;
}
@@ -370,7 +380,7 @@ out:
err:
hardif_free_ref(hard_iface);
- return -ENOMEM;
+ return ret;
}
void hardif_disable_interface(struct hard_iface *hard_iface)
diff --git a/batman-adv/soft-interface.c b/batman-adv/soft-interface.c
index 6b514ec..9ed2614 100644
--- a/batman-adv/soft-interface.c
+++ b/batman-adv/soft-interface.c
@@ -622,6 +622,19 @@ void softif_destroy(struct net_device *soft_iface)
unregister_netdevice(soft_iface);
}
+int softif_is_valid(struct net_device *net_dev)
+{
+#ifdef HAVE_NET_DEVICE_OPS
+ if (net_dev->netdev_ops->ndo_start_xmit == interface_tx)
+ return 1;
+#else
+ if (net_dev->hard_start_xmit == interface_tx)
+ return 1;
+#endif
+
+ return 0;
+}
+
/* ethtool */
static int bat_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
diff --git a/batman-adv/soft-interface.h b/batman-adv/soft-interface.h
index 80a3607..4789b6f 100644
--- a/batman-adv/soft-interface.h
+++ b/batman-adv/soft-interface.h
@@ -31,5 +31,6 @@ void interface_rx(struct net_device *soft_iface,
int hdr_size);
struct net_device *softif_create(char *name);
void softif_destroy(struct net_device *soft_iface);
+int softif_is_valid(struct net_device *net_dev);
#endif /* _NET_BATMAN_ADV_SOFT_INTERFACE_H_ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device
2011-03-03 21:08 ` [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device Sven Eckelmann
@ 2011-03-04 20:55 ` Marek Lindner
2011-03-04 21:18 ` Linus Lüssing
2011-03-04 21:34 ` Marek Lindner
1 sibling, 1 reply; 9+ messages in thread
From: Marek Lindner @ 2011-03-04 20:55 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Thursday 03 March 2011 22:08:39 Sven Eckelmann wrote:
> When trying to associate a net_device with another net_device which
> already exists, batman-adv assumes that this interface is a fully
> initialized batman mesh interface without checking it. The behaviour
> when accessing data behind netdev_priv of a random net_device is
> undefined and potentially dangerous.
Linus, can you confirm this patch fixes the problem for you ?
Thanks,
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device
2011-03-04 20:55 ` Marek Lindner
@ 2011-03-04 21:18 ` Linus Lüssing
2011-03-04 21:24 ` Marek Lindner
0 siblings, 1 reply; 9+ messages in thread
From: Linus Lüssing @ 2011-03-04 21:18 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Fri, Mar 04, 2011 at 09:55:12PM +0100, Marek Lindner wrote:
> On Thursday 03 March 2011 22:08:39 Sven Eckelmann wrote:
> > When trying to associate a net_device with another net_device which
> > already exists, batman-adv assumes that this interface is a fully
> > initialized batman mesh interface without checking it. The behaviour
> > when accessing data behind netdev_priv of a random net_device is
> > undefined and potentially dangerous.
>
> Linus, can you confirm this patch fixes the problem for you ?
>
> Thanks,
> Marek
>
Yep, works as expected, thanks Sven!
"Can't create batman mesh interface interface %s:"
-> there's one 'interface' too much though
Cheers, Linus
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device
2011-03-04 21:18 ` Linus Lüssing
@ 2011-03-04 21:24 ` Marek Lindner
0 siblings, 0 replies; 9+ messages in thread
From: Marek Lindner @ 2011-03-04 21:24 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Friday 04 March 2011 22:18:44 Linus Lüssing wrote:
> Yep, works as expected, thanks Sven!
Ok.
> "Can't create batman mesh interface interface %s:"
> -> there's one 'interface' too much though
I will fix that before committing the patch.
Cheers,
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device
2011-03-03 21:08 ` [B.A.T.M.A.N.] [PATCHv2] batman-adv: Disallow regular interface as mesh device Sven Eckelmann
2011-03-04 20:55 ` Marek Lindner
@ 2011-03-04 21:34 ` Marek Lindner
1 sibling, 0 replies; 9+ messages in thread
From: Marek Lindner @ 2011-03-04 21:34 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Thursday 03 March 2011 22:08:39 Sven Eckelmann wrote:
> When trying to associate a net_device with another net_device which
> already exists, batman-adv assumes that this interface is a fully
> initialized batman mesh interface without checking it. The behaviour
> when accessing data behind netdev_priv of a random net_device is
> undefined and potentially dangerous.
Applied in revision 1955.
Thanks,
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread