hlist_add_head_rcu must be protected using gw_list_lock of the current bat_priv like already done with hlist_del_rcu. It is important that this lock is now always done using spin_lock_irqsave because gw_node_add can also be called indirectly from parts of the kernel with interrupts disabled. Signed-off-by: Sven Eckelmann <sven.eckelmann(a)gmx.de> --- batman-adv/gateway_client.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/batman-adv/gateway_client.c b/batman-adv/gateway_client.c index 6721398..dd96d99 100644 --- a/batman-adv/gateway_client.c +++ b/batman-adv/gateway_client.c @@ -196,6 +196,7 @@ static void gw_node_add(struct bat_priv *bat_priv, { struct gw_node *gw_node; int down, up; + unsigned long flags; gw_node = kmalloc(sizeof(struct gw_node), GFP_ATOMIC); if (!gw_node) @@ -205,7 +206,9 @@ static void gw_node_add(struct bat_priv *bat_priv, INIT_HLIST_NODE(&gw_node->list); gw_node->orig_node = orig_node; + spin_lock_irqsave(&bat_priv->gw_list_lock, flags); hlist_add_head_rcu(&gw_node->list, &bat_priv->gw_list); + spin_unlock_irqrestore(&bat_priv->gw_list_lock, flags); gw_srv_class_to_kbit(new_gwflags, &down, &up); bat_dbg(DBG_BATMAN, bat_priv, @@ -273,8 +276,9 @@ void gw_node_purge_deleted(struct bat_priv *bat_priv) struct gw_node *gw_node; struct hlist_node *node, *node_tmp; unsigned long timeout = 2 * PURGE_TIMEOUT * HZ; + unsigned long flags; - spin_lock(&bat_priv->gw_list_lock); + spin_lock_irqsave(&bat_priv->gw_list_lock, flags); hlist_for_each_entry_safe(gw_node, node, node_tmp, &bat_priv->gw_list, list) { @@ -286,15 +290,16 @@ void gw_node_purge_deleted(struct bat_priv *bat_priv) } } - spin_unlock(&bat_priv->gw_list_lock); + spin_unlock_irqrestore(&bat_priv->gw_list_lock, flags); } void gw_node_list_free(struct bat_priv *bat_priv) { struct gw_node *gw_node; struct hlist_node *node, *node_tmp; + unsigned long flags; - spin_lock(&bat_priv->gw_list_lock); + spin_lock_irqsave(&bat_priv->gw_list_lock, flags); hlist_for_each_entry_safe(gw_node, node, node_tmp, &bat_priv->gw_list, list) { @@ -303,7 +308,7 @@ void gw_node_list_free(struct bat_priv *bat_priv) } gw_deselect(bat_priv); - spin_unlock(&bat_priv->gw_list_lock); + spin_unlock_irqrestore(&bat_priv->gw_list_lock, flags); } static int _write_buffer_text(struct bat_priv *bat_priv, -- 1.7.2.3

Regions which do not use rcu functions don't need to protected by rcu_read_lock. If we want to protect data from being freed than it must be covered by the same read-side critical section or otherwise the grace period may already ended and freed the memory before we called rcu_read_lock again. Signed-off-by: Sven Eckelmann <sven.eckelmann(a)gmx.de> --- batman-adv/gateway_client.c | 4 ---- batman-adv/originator.c | 2 -- 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/batman-adv/gateway_client.c b/batman-adv/gateway_client.c index dd96d99..bfac0ff 100644 --- a/batman-adv/gateway_client.c +++ b/batman-adv/gateway_client.c @@ -342,9 +342,7 @@ int gw_client_seq_print_text(struct seq_file *seq, void *offset) struct hlist_node *node; int gw_count = 0; - rcu_read_lock(); if (!bat_priv->primary_if) { - rcu_read_unlock(); return seq_printf(seq, "BATMAN mesh %s disabled - please " "specify interfaces to enable it\n", @@ -352,7 +350,6 @@ int gw_client_seq_print_text(struct seq_file *seq, void *offset) } if (bat_priv->primary_if->if_status != IF_ACTIVE) { - rcu_read_unlock(); return seq_printf(seq, "BATMAN mesh %s disabled - " "primary interface not active\n", @@ -365,7 +362,6 @@ int gw_client_seq_print_text(struct seq_file *seq, void *offset) "outgoingIF", SOURCE_VERSION, REVISION_VERSION_STR, bat_priv->primary_if->net_dev->name, bat_priv->primary_if->addr_str, net_dev->name); - rcu_read_unlock(); rcu_read_lock(); hlist_for_each_entry_rcu(gw_node, node, &bat_priv->gw_list, list) { diff --git a/batman-adv/originator.c b/batman-adv/originator.c index 2250266..3424ac2 100644 --- a/batman-adv/originator.c +++ b/batman-adv/originator.c @@ -335,7 +335,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset) net_dev->name); } - rcu_read_lock(); seq_printf(seq, "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%s (%s)]\n", SOURCE_VERSION, REVISION_VERSION_STR, bat_priv->primary_if->net_dev->name, @@ -343,7 +342,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset) seq_printf(seq, " %-15s %s (%s/%i) %17s [%10s]: %20s ...\n", "Originator", "last-seen", "#", TQ_MAX_VALUE, "Nexthop", "outgoingIF", "Potential nexthops"); - rcu_read_unlock(); spin_lock_irqsave(&bat_priv->orig_hash_lock, flags); -- 1.7.2.3

gw_election may leak data from the rcu protected list of all gateway nodes outside the read-side critical area. This is not valid as we may free the data using a call_rcu created callback after we unlock using rcu_read_unlock. A workaround is to provide a reference count to be sure that the memory isn't freed to early. It is currently only to implement the already existing functionality and doesn't provide the full tracking of all usage cases. Additionally, we must gw_node_hold inside the rcu_read_lock()..rcu_read_unlock() before we attach to the structure which "leaks" it. When another function now removed it from its usage context (curr_gw, usage on stack, ...) then we must gw_node_put it. If it is decremented to zero then we can issue the call_rcu to the freeing function. So "put" is not allowed inside an rcu_read_lock. Signed-off-by: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; --- batman-adv/gateway_client.c | 17 +++++++++++++++-- batman-adv/types.h | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/batman-adv/gateway_client.c b/batman-adv/gateway_client.c index 8bc1cb0..16f0757 100644 --- a/batman-adv/gateway_client.c +++ b/batman-adv/gateway_client.c @@ -28,6 +28,17 @@ #include <linux/udp.h> #include <linux/if_vlan.h> +static void gw_node_hold(struct gw_node *gw_node) +{ + atomic_inc(&gw_node->refcnt); +} + +static void gw_node_put(struct gw_node *gw_node) +{ + if (atomic_dec_and_test(&gw_node->refcnt)) + kfree(gw_node); +} + void *gw_get_selected(struct bat_priv *bat_priv) { struct gw_node *curr_gateway_tmp = bat_priv->curr_gw; @@ -205,6 +216,8 @@ static void gw_node_add(struct bat_priv *bat_priv, memset(gw_node, 0, sizeof(struct gw_node)); INIT_HLIST_NODE(&gw_node->list); gw_node->orig_node = orig_node; + atomic_set(&gw_node->refcnt, 0); + gw_node_hold(gw_node); spin_lock_irqsave(&bat_priv->gw_list_lock, flags); hlist_add_head_rcu(&gw_node->list, &bat_priv->gw_list); @@ -281,7 +294,7 @@ void gw_node_purge_deleted(struct bat_priv *bat_priv) hlist_del_rcu(&gw_node->list); synchronize_rcu(); - kfree(gw_node); + gw_node_put(gw_node); } } @@ -300,7 +313,7 @@ void gw_node_list_free(struct bat_priv *bat_priv) &bat_priv->gw_list, list) { hlist_del_rcu(&gw_node->list); synchronize_rcu(); - kfree(gw_node); + gw_node_put(gw_node); } gw_deselect(bat_priv); diff --git a/batman-adv/types.h b/batman-adv/types.h index 1940404..ecc4365 100644 --- a/batman-adv/types.h +++ b/batman-adv/types.h @@ -95,6 +95,7 @@ struct gw_node { struct hlist_node list; struct orig_node *orig_node; unsigned long deleted; + atomic_t refcnt; }; /** -- 1.7.2.3

We reference the gw_nodes as curr_gw for each bat_priv. They are tracked inside a rcu protected list gw_list which may free the memory after it gets removed from the list. Nevertheless it could still be referenced by curr_gw and access to it would result in a kernel oops. Signed-off-by: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; --- batman-adv/gateway_client.c | 22 +++++++++++++++++++--- 1 files changed, 19 insertions(+), 3 deletions(-) diff --git a/batman-adv/gateway_client.c b/batman-adv/gateway_client.c index 16f0757..1cad4f8 100644 --- a/batman-adv/gateway_client.c +++ b/batman-adv/gateway_client.c @@ -51,13 +51,18 @@ void *gw_get_selected(struct bat_priv *bat_priv) void gw_deselect(struct bat_priv *bat_priv) { + struct gw_node *gw_node = bat_priv->curr_gw; + bat_priv->curr_gw = NULL; + + if (gw_node) + gw_node_put(gw_node); } void gw_election(struct bat_priv *bat_priv) { struct hlist_node *node; - struct gw_node *gw_node, *curr_gw_tmp = NULL; + struct gw_node *gw_node, *curr_gw_tmp = NULL, *old_gw_node = NULL; uint8_t max_tq = 0; uint32_t max_gw_factor = 0, tmp_gw_factor = 0; int down, up; @@ -131,7 +136,6 @@ void gw_election(struct bat_priv *bat_priv) if (tmp_gw_factor > max_gw_factor) max_gw_factor = tmp_gw_factor; } - rcu_read_unlock(); if (bat_priv->curr_gw != curr_gw_tmp) { if ((bat_priv->curr_gw) && (!curr_gw_tmp)) @@ -153,8 +157,17 @@ void gw_election(struct bat_priv *bat_priv) curr_gw_tmp->orig_node->gw_flags, curr_gw_tmp->orig_node->router->tq_avg); + old_gw_node = bat_priv->curr_gw; + if (curr_gw_tmp) + gw_node_hold(curr_gw_tmp); + bat_priv->curr_gw = curr_gw_tmp; } + + rcu_read_unlock(); + + if (old_gw_node) + gw_node_put(old_gw_node); } void gw_check_election(struct bat_priv *bat_priv, struct orig_node *orig_node) @@ -258,8 +271,11 @@ void gw_node_update(struct bat_priv *bat_priv, "Gateway %pM removed from gateway list\n", orig_node->orig); - if (gw_node == bat_priv->curr_gw) + if (gw_node == bat_priv->curr_gw) { + rcu_read_unlock(); gw_deselect(bat_priv); + return; + } } rcu_read_unlock(); -- 1.7.2.3

The return of get_batman_if_by_netdev and get_active_batman_if leaks a pointer from the rcu protected list of interfaces. We must protect it to prevent a too early release of the memory. Those functions must increase the reference counter before rcu_read_unlock or it may be to late to prevent a free. hardif_add_interface must also increase the reference count for the returned batman_if to make the behaviour consistent. Reported-by: Paul E. McKenney &lt;paulmck(a)linux.vnet.ibm.com&gt; Signed-off-by: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; --- Forgot some breaks in a switch statements. batman-adv/bat_sysfs.c | 42 ++++++++++++++++++++++++++++++++---------- batman-adv/hard-interface.c | 29 ++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/batman-adv/bat_sysfs.c b/batman-adv/bat_sysfs.c index 8e180ba..9ab2bfe 100644 --- a/batman-adv/bat_sysfs.c +++ b/batman-adv/bat_sysfs.c @@ -453,13 +453,17 @@ static ssize_t show_mesh_iface(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); + ssize_t length; if (!batman_if) return 0; - return sprintf(buff, "%s\n", - batman_if->if_status == IF_NOT_IN_USE ? - "none" : batman_if->soft_iface->name); + length = sprintf(buff, "%s\n", batman_if->if_status == IF_NOT_IN_USE ? + "none" : batman_if->soft_iface->name); + + hardif_put(batman_if); + + return length; } static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, @@ -469,6 +473,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); int status_tmp = -1; + int ret; if (!batman_if) return count; @@ -479,6 +484,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, if (strlen(buff) >= IFNAMSIZ) { pr_err("Invalid parameter for 'mesh_iface' setting received: " "interface name too long '%s'\n", buff); + hardif_put(batman_if); return -EINVAL; } @@ -488,13 +494,16 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, status_tmp = IF_I_WANT_YOU; if ((batman_if->if_status == status_tmp) || ((batman_if->soft_iface) && - (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0))) + (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0))) { + hardif_put(batman_if); return count; + } if (status_tmp == IF_NOT_IN_USE) { rtnl_lock(); hardif_disable_interface(batman_if); rtnl_unlock(); + hardif_put(batman_if); return count; } @@ -505,7 +514,10 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, rtnl_unlock(); } - return hardif_enable_interface(batman_if, buff); + ret = hardif_enable_interface(batman_if, buff); + hardif_put(batman_if); + + return ret; } static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr, @@ -514,23 +526,33 @@ static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); + ssize_t length; if (!batman_if) return 0; switch (batman_if->if_status) { case IF_TO_BE_REMOVED: - return sprintf(buff, "disabling\n"); + length = sprintf(buff, "disabling\n"); + break; case IF_INACTIVE: - return sprintf(buff, "inactive\n"); + length = sprintf(buff, "inactive\n"); + break; case IF_ACTIVE: - return sprintf(buff, "active\n"); + length = sprintf(buff, "active\n"); + break; case IF_TO_BE_ACTIVATED: - return sprintf(buff, "enabling\n"); + length = sprintf(buff, "enabling\n"); + break; case IF_NOT_IN_USE: default: - return sprintf(buff, "not in use\n"); + length = sprintf(buff, "not in use\n"); + break; } + + hardif_put(batman_if); + + return length; } static BAT_ATTR(mesh_iface, S_IRUGO | S_IWUSR, diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c index 445498c..f519b4b 100644 --- a/batman-adv/hard-interface.c +++ b/batman-adv/hard-interface.c @@ -51,6 +51,9 @@ struct batman_if *get_batman_if_by_netdev(struct net_device *net_dev) batman_if = NULL; out: + if (batman_if) + hardif_hold(batman_if); + rcu_read_unlock(); return batman_if; } @@ -98,6 +101,9 @@ static struct batman_if *get_active_batman_if(struct net_device *soft_iface) batman_if = NULL; out: + if (batman_if) + hardif_hold(batman_if); + rcu_read_unlock(); return batman_if; } @@ -294,6 +300,7 @@ int hardif_enable_interface(struct batman_if *batman_if, char *iface_name) batman_if->batman_adv_ptype.type = __constant_htons(ETH_P_BATMAN); batman_if->batman_adv_ptype.func = batman_skb_recv; batman_if->batman_adv_ptype.dev = batman_if->net_dev; + hardif_hold(batman_if); dev_add_pack(&batman_if->batman_adv_ptype); atomic_set(&batman_if->seqno, 1); @@ -352,13 +359,20 @@ void hardif_disable_interface(struct batman_if *batman_if) bat_info(batman_if->soft_iface, "Removing interface: %s\n", batman_if->net_dev->name); dev_remove_pack(&batman_if->batman_adv_ptype); + hardif_put(batman_if); bat_priv->num_ifaces--; orig_hash_del_if(batman_if, bat_priv->num_ifaces); - if (batman_if == bat_priv->primary_if) - set_primary_if(bat_priv, - get_active_batman_if(batman_if->soft_iface)); + if (batman_if == bat_priv->primary_if) { + struct batman_if *new_if; + + new_if = get_active_batman_if(batman_if->soft_iface); + set_primary_if(bat_priv, new_if); + + if (new_if) + hardif_put(new_if); + } kfree(batman_if->packet_buff); batman_if->packet_buff = NULL; @@ -412,6 +426,8 @@ static struct batman_if *hardif_add_interface(struct net_device *net_dev) list_add_tail_rcu(&batman_if->list, &if_list); spin_unlock(&if_list_lock); + /* extra reference for return */ + hardif_hold(batman_if); return batman_if; free_if: @@ -461,7 +477,7 @@ static int hard_if_event(struct notifier_block *this, struct bat_priv *bat_priv; if (!batman_if && event == NETDEV_REGISTER) - batman_if = hardif_add_interface(net_dev); + batman_if = hardif_add_interface(net_dev); if (!batman_if) goto out; @@ -484,8 +500,10 @@ static int hard_if_event(struct notifier_block *this, update_min_mtu(batman_if->soft_iface); break; case NETDEV_CHANGEADDR: - if (batman_if->if_status == IF_NOT_IN_USE) + if (batman_if->if_status == IF_NOT_IN_USE) { + hardif_put(batman_if); goto out; + } check_known_mac_addr(batman_if->net_dev->dev_addr); update_mac_addresses(batman_if); @@ -497,6 +515,7 @@ static int hard_if_event(struct notifier_block *this, default: break; }; + hardif_put(batman_if); out: return NOTIFY_DONE; -- 1.7.2.3

From: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; The return of get_batman_if_by_netdev and get_active_batman_if leaks a pointer from the rcu protected list of interfaces. We must protect it to prevent a too early release of the memory. Those functions must increase the reference counter before rcu_read_unlock or it may be to late to prevent a free. hardif_add_interface must also increase the reference count for the returned batman_if to make the behaviour consistent. Reported-by: Paul E. McKenney &lt;paulmck(a)linux.vnet.ibm.com&gt; Signed-off-by: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; Signed-off-by: Marek Lindner &lt;lindner_marek(a)yahoo.de&gt; --- batman-adv/bat_sysfs.c | 42 +++++++++++++++++++++++++++++++--------- batman-adv/hard-interface.c | 44 +++++++++++++++++++++++++++++++++--------- 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/batman-adv/bat_sysfs.c b/batman-adv/bat_sysfs.c index 8e180ba..9ab2bfe 100644 --- a/batman-adv/bat_sysfs.c +++ b/batman-adv/bat_sysfs.c @@ -453,13 +453,17 @@ static ssize_t show_mesh_iface(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); + ssize_t length; if (!batman_if) return 0; - return sprintf(buff, "%s\n", - batman_if->if_status == IF_NOT_IN_USE ? - "none" : batman_if->soft_iface->name); + length = sprintf(buff, "%s\n", batman_if->if_status == IF_NOT_IN_USE ? + "none" : batman_if->soft_iface->name); + + hardif_put(batman_if); + + return length; } static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, @@ -469,6 +473,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); int status_tmp = -1; + int ret; if (!batman_if) return count; @@ -479,6 +484,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, if (strlen(buff) >= IFNAMSIZ) { pr_err("Invalid parameter for 'mesh_iface' setting received: " "interface name too long '%s'\n", buff); + hardif_put(batman_if); return -EINVAL; } @@ -488,13 +494,16 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, status_tmp = IF_I_WANT_YOU; if ((batman_if->if_status == status_tmp) || ((batman_if->soft_iface) && - (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0))) + (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0))) { + hardif_put(batman_if); return count; + } if (status_tmp == IF_NOT_IN_USE) { rtnl_lock(); hardif_disable_interface(batman_if); rtnl_unlock(); + hardif_put(batman_if); return count; } @@ -505,7 +514,10 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr, rtnl_unlock(); } - return hardif_enable_interface(batman_if, buff); + ret = hardif_enable_interface(batman_if, buff); + hardif_put(batman_if); + + return ret; } static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr, @@ -514,23 +526,33 @@ static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr, struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct batman_if *batman_if = get_batman_if_by_netdev(net_dev); + ssize_t length; if (!batman_if) return 0; switch (batman_if->if_status) { case IF_TO_BE_REMOVED: - return sprintf(buff, "disabling\n"); + length = sprintf(buff, "disabling\n"); + break; case IF_INACTIVE: - return sprintf(buff, "inactive\n"); + length = sprintf(buff, "inactive\n"); + break; case IF_ACTIVE: - return sprintf(buff, "active\n"); + length = sprintf(buff, "active\n"); + break; case IF_TO_BE_ACTIVATED: - return sprintf(buff, "enabling\n"); + length = sprintf(buff, "enabling\n"); + break; case IF_NOT_IN_USE: default: - return sprintf(buff, "not in use\n"); + length = sprintf(buff, "not in use\n"); + break; } + + hardif_put(batman_if); + + return length; } static BAT_ATTR(mesh_iface, S_IRUGO | S_IWUSR, diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c index 445498c..64ff53d 100644 --- a/batman-adv/hard-interface.c +++ b/batman-adv/hard-interface.c @@ -51,6 +51,9 @@ struct batman_if *get_batman_if_by_netdev(struct net_device *net_dev) batman_if = NULL; out: + if (batman_if) + hardif_hold(batman_if); + rcu_read_unlock(); return batman_if; } @@ -98,31 +101,45 @@ static struct batman_if *get_active_batman_if(struct net_device *soft_iface) batman_if = NULL; out: + if (batman_if) + hardif_hold(batman_if); + rcu_read_unlock(); return batman_if; } +static void update_primary_addr(struct bat_priv *bat_priv) +{ + struct vis_packet *vis_packet; + + vis_packet = (struct vis_packet *) + bat_priv->my_vis_info->skb_packet->data; + memcpy(vis_packet->vis_orig, + bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN); + memcpy(vis_packet->sender_orig, + bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN); +} + static void set_primary_if(struct bat_priv *bat_priv, struct batman_if *batman_if) { struct batman_packet *batman_packet; - struct vis_packet *vis_packet; + + if (bat_priv->primary_if) + hardif_put(bat_priv->primary_if); bat_priv->primary_if = batman_if; if (!bat_priv->primary_if) return; + hardif_hold(bat_priv->primary_if); + batman_packet = (struct batman_packet *)(batman_if->packet_buff); batman_packet->flags = PRIMARIES_FIRST_HOP; batman_packet->ttl = TTL; - vis_packet = (struct vis_packet *) - bat_priv->my_vis_info->skb_packet->data; - memcpy(vis_packet->vis_orig, - bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN); - memcpy(vis_packet->sender_orig, - bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN); + update_primary_addr(bat_priv); /*** * hacky trick to make sure that we send the HNA information via @@ -294,6 +311,7 @@ int hardif_enable_interface(struct batman_if *batman_if, char *iface_name) batman_if->batman_adv_ptype.type = __constant_htons(ETH_P_BATMAN); batman_if->batman_adv_ptype.func = batman_skb_recv; batman_if->batman_adv_ptype.dev = batman_if->net_dev; + hardif_hold(batman_if); dev_add_pack(&batman_if->batman_adv_ptype); atomic_set(&batman_if->seqno, 1); @@ -352,6 +370,7 @@ void hardif_disable_interface(struct batman_if *batman_if) bat_info(batman_if->soft_iface, "Removing interface: %s\n", batman_if->net_dev->name); dev_remove_pack(&batman_if->batman_adv_ptype); + hardif_put(batman_if); bat_priv->num_ifaces--; orig_hash_del_if(batman_if, bat_priv->num_ifaces); @@ -412,6 +431,8 @@ static struct batman_if *hardif_add_interface(struct net_device *net_dev) list_add_tail_rcu(&batman_if->list, &if_list); spin_unlock(&if_list_lock); + /* extra reference for return */ + hardif_hold(batman_if); return batman_if; free_if: @@ -461,7 +482,7 @@ static int hard_if_event(struct notifier_block *this, struct bat_priv *bat_priv; if (!batman_if && event == NETDEV_REGISTER) - batman_if = hardif_add_interface(net_dev); + batman_if = hardif_add_interface(net_dev); if (!batman_if) goto out; @@ -484,19 +505,22 @@ static int hard_if_event(struct notifier_block *this, update_min_mtu(batman_if->soft_iface); break; case NETDEV_CHANGEADDR: - if (batman_if->if_status == IF_NOT_IN_USE) + if (batman_if->if_status == IF_NOT_IN_USE) { + hardif_put(batman_if); goto out; + } check_known_mac_addr(batman_if->net_dev->dev_addr); update_mac_addresses(batman_if); bat_priv = netdev_priv(batman_if->soft_iface); if (batman_if == bat_priv->primary_if) - set_primary_if(bat_priv, batman_if); + update_primary_addr(bat_priv); break; default: break; }; + hardif_put(batman_if); out: return NOTIFY_DONE; -- 1.7.1

set_primary_if exchanges the current primary interfaces with a new one. This is a new reference and thus we have to count it and decrease the count of the old primary interface. Signed-off-by: Sven Eckelmann &lt;sven.eckelmann(a)gmx.de&gt; --- I would like to keep that order of hold -> exchange -> put for later changes (rcu or something like that for primary_if and curr_gw). batman-adv/hard-interface.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c index f519b4b..942a44a 100644 --- a/batman-adv/hard-interface.c +++ b/batman-adv/hard-interface.c @@ -113,9 +113,17 @@ static void set_primary_if(struct bat_priv *bat_priv, { struct batman_packet *batman_packet; struct vis_packet *vis_packet; + struct batman_if *old_if; + if (batman_if) + hardif_hold(batman_if); + + old_if = bat_priv->primary_if; bat_priv->primary_if = batman_if; + if (old_if) + hardif_put(old_if); + if (!bat_priv->primary_if) return; -- 1.7.2.3