Project

General

Profile

Bug #244

Updated by Sven Eckelmann over 1 year ago

"Andrew detected one instance of a kref_get_unless_zero":https://lists.open-mesh.org/mailman3/hyperkitty/list/b.a.t.m.a.n@lists.open-mesh.org/message/CLLUZNQDZKLLPSRZTTKIB3GDE2OTFTMF/ kref_get_unless_zero":https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-March/014588.html which didn't seem to belong to a rcu_read_lock protected section. I went through the rest of the code and annotated things which look similar.

<pre>
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 2c65668..a27fda0 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -679,9 +679,11 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
unsigned char *skb_buff;
unsigned int skb_size;

+ /* TODO */
if (!kref_get_unless_zero(&if_incoming->refcount))
return;

+ /* TODO */
if (!kref_get_unless_zero(&if_outgoing->refcount))
goto out_free_incoming;

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index c59aff5..fafc053 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -135,6 +135,7 @@ static void batadv_gw_select(struct batadv_priv *bat_priv,

spin_lock_bh(&bat_priv->gw.list_lock);

+ /* TODO */
if (new_gw_node && !kref_get_unless_zero(&new_gw_node->refcount))
new_gw_node = NULL;

@@ -440,6 +441,7 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv,
if (gateway->bandwidth_down == 0)
return;

+ /* TODO */
if (!kref_get_unless_zero(&orig_node->refcount))
return;

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 3240a67..a4c488d 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -236,6 +236,7 @@ static void batadv_primary_if_select(struct batadv_priv *bat_priv,

ASSERT_RTNL();

+ /* TODO */
if (new_hard_iface && !kref_get_unless_zero(&new_hard_iface->refcount))
new_hard_iface = NULL;

@@ -464,6 +465,7 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
if (hard_iface->if_status != BATADV_IF_NOT_IN_USE)
goto out;

+ /* TODO */
if (!kref_get_unless_zero(&hard_iface->refcount))
goto out;

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index d64ddb9..89bc5b1 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -736,6 +736,7 @@ static struct batadv_tvlv_container
if (tvlv_tmp->tvlv_hdr.version != version)
continue;

+ /* TODO */
if (!kref_get_unless_zero(&tvlv_tmp->refcount))
continue;

diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index 32f9fa1..e6dc208 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -856,6 +856,7 @@ static struct batadv_nc_node
if (!nc_node)
return NULL;

+ /* TODO */
if (!kref_get_unless_zero(&orig_neigh_node->refcount))
goto free;

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index e63d6a5..92e6a41 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -381,6 +381,7 @@ batadv_orig_ifinfo_new(struct batadv_orig_node *orig_node,
if (!orig_ifinfo)
goto out;

+ /* TODO */
if (if_outgoing != BATADV_IF_DEFAULT &&
!kref_get_unless_zero(&if_outgoing->refcount)) {
kfree(orig_ifinfo);
@@ -462,6 +463,7 @@ batadv_neigh_ifinfo_new(struct batadv_neigh_node *neigh,
if (!neigh_ifinfo)
goto out;

+ /* TODO */
if (if_outgoing && !kref_get_unless_zero(&if_outgoing->refcount)) {
kfree(neigh_ifinfo);
neigh_ifinfo = NULL;
@@ -539,6 +541,7 @@ batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
if (hardif_neigh)
goto out;

+ /* TODO */
if (!kref_get_unless_zero(&hard_iface->refcount))
goto out;

@@ -650,6 +653,7 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node,
if (!neigh_node)
goto out;

+ /* TODO */
if (!kref_get_unless_zero(&hard_iface->refcount)) {
kfree(neigh_node);
neigh_node = NULL;
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 45093c6..39a2e04 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -101,6 +101,7 @@ static void _batadv_update_route(struct batadv_priv *bat_priv,
batadv_neigh_node_put(curr_router);

/* increase refcount of new best neighbor */
+ /* TODO */
if (neigh_node && !kref_get_unless_zero(&neigh_node->refcount))
neigh_node = NULL;


</pre>

"My proposal":https://lists.open-mesh.org/mailman3/hyperkitty/list/b.a.t.m.a.n@lists.open-mesh.org/message/ZJL4NCMJ6KPDAUDZG4MXVGO5ABQYL2MG/ proposal":https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-March/014591.html was to use kref_get when we already have a valid reference and to move the kref_get_unless_zero up to the rcu_read_lock section code (when there is one). This has the benefit that kref_get will warn us when the reference counting broke end we try to get a reference when the reference counter already reached zero (which should not happen in this situation).

Back