From patchwork Sat Sep 18 14:42:35 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Lindner X-Patchwork-Id: 430 Return-Path: Received: from smtp142.mail.ukl.yahoo.com (smtp142.mail.ukl.yahoo.com [77.238.184.73]) by open-mesh.org (Postfix) with SMTP id AB53215430E for ; Sat, 18 Sep 2010 16:43:47 +0200 (CEST) Received: (qmail 83954 invoked from network); 18 Sep 2010 14:43:46 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.de; h=DKIM-Signature:Received:X-Yahoo-SMTP:X-YMail-OSG:X-Yahoo-Newman-Property:From:To:Cc:Subject:Date:Message-Id:X-Mailer:In-Reply-To:References; b=2JKcwtzvhrEpopUAd+VkVRh18DqJfFU/TPvX7szHxnLgOjFyBoVLLzOnx63r1lMYbYkSLzk3zvEpJzHH3ZSIuhFSp3tPSR0KCwyL2fRC2GyguH89N2AiBRwm+noA3xfWUFpT65sr8bKeaxyvu5Znm3kVbmVryAjjGs2P1yrVhy8= ; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.de; s=s1024; t=1284821026; bh=y1DOKOtUXenlZzbR3QV77XTGBJSgUJhGCWrYs9eBtSw=; h=Received:X-Yahoo-SMTP:X-YMail-OSG:X-Yahoo-Newman-Property:From:To:Cc:Subject:Date:Message-Id:X-Mailer:In-Reply-To:References; b=BRJENOCXk7dL74txjfa+qfke13c1NgKeswwmAMbzVWo/xAUcNiWmAhJagD1TybfjYmaYIO1Q53+PMk9uI2uL4uUQGqbOHn+bsjD0GE3q/Hz55HRo85ZzuPjOxq45/P/VG8rTQOM40a6L5RMfQgXhK5i/4wTHLTpdRZLvNQ8KyJY= Received: from localhost (lindner_marek@78.225.40.81 with plain) by smtp142.mail.ukl.yahoo.com with SMTP; 18 Sep 2010 14:43:45 +0000 GMT X-Yahoo-SMTP: tW.h3tiswBBMXO2coYcbPigGD5Lt6zY_.Zc- X-YMail-OSG: .aTmfn8VM1mEF3ykEdmdjCi3roSN54fOTvwrhLNxld9rbUY Yl1x0pIL0wZQ_uFIe5WLMdjqT589I2hB.qgu1dnT6lfl1hVHxJR6A4DEln52 1jgvRxrkxlXD5JfpbvE2bxhn8UEomP11hRIY5d7ZGgD4wj9ntpJINpAzcQON VQ8sdBCemD4FmjWw3NIA1DwOLSKAeDTrLc1VWTq5ew_9vIbdeUn3wFVT6ntB OVx6nl.BNvcMB.PasIUmBqqqQKhfGNHyaxDRdJRDMxm4d9bcrf2BfHCv4VlP lgryfh9g- X-Yahoo-Newman-Property: ymail-3 From: Marek Lindner To: b.a.t.m.a.n@lists.open-mesh.org Date: Sat, 18 Sep 2010 16:42:35 +0200 Message-Id: <1284820955-23182-1-git-send-email-lindner_marek@yahoo.de> X-Mailer: git-send-email 1.7.1 In-Reply-To: <201009181642.08297.lindner_marek@yahoo.de> References: <201009181642.08297.lindner_marek@yahoo.de> Cc: Marek Lindner Subject: [B.A.T.M.A.N.] [PATCH] batman-adv: count batman_if list queries as reference X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Sep 2010 14:43:48 -0000 From: Sven Eckelmann 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 Signed-off-by: Sven Eckelmann Signed-off-by: Marek Lindner --- 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;