Hi,
Here are fixes for the bugs introduced by the last fixes to the capability checks.
Cheers, Linus
PS: For internal things where the bit position isn't important (as in this case), I think doing a "= BIT(x)" assignments only adds noise. For packet format things where the bit position is more interesting, it might be good to keep "= BIT(x)" as it gives O(1) access to the organic reader ;).
~~~~~
Changelog v2: * Split into 4 individual patches * Using test_bit() instead of BIT()
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: bfd0fbaef270 ("batman-adv: Make DAT capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- distributed-arp-table.c | 2 +- types.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b2cc19b..c0c514d 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -422,7 +422,7 @@ static bool batadv_is_orig_node_eligible(struct batadv_dat_candidate *res, int j;
/* check if orig node candidate is running DAT */ - if (!(candidate->capabilities & BATADV_ORIG_CAPA_HAS_DAT)) + if (!(test_bit(BATADV_ORIG_CAPA_HAS_DAT, &candidate->capabilities))) goto out;
/* Check if this node has already been selected... */ diff --git a/types.h b/types.h index 65dc6bf..08a6343 100644 --- a/types.h +++ b/types.h @@ -299,7 +299,7 @@ struct batadv_orig_node { * (= orig node announces a tvlv of type BATADV_TVLV_MCAST) */ enum batadv_orig_capabilities { - BATADV_ORIG_CAPA_HAS_DAT = BIT(0), + BATADV_ORIG_CAPA_HAS_DAT, BATADV_ORIG_CAPA_HAS_NC = BIT(1), BATADV_ORIG_CAPA_HAS_TT = BIT(2), BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
On Friday, July 03, 2015 18:29:56 Linus Lüssing wrote:
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: bfd0fbaef270 ("batman-adv: Make DAT capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
distributed-arp-table.c | 2 +- types.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Applied in revision de46667.
Thanks, Marek
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: 586df9e2537b ("batman-adv: Make NC capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- network-coding.c | 2 +- types.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/network-coding.c b/network-coding.c index 3ce493e..0309c2c 100644 --- a/network-coding.c +++ b/network-coding.c @@ -871,7 +871,7 @@ void batadv_nc_update_nc_node(struct batadv_priv *bat_priv, goto out;
/* check if orig node is network coding enabled */ - if (!(orig_node->capabilities & BATADV_ORIG_CAPA_HAS_NC)) + if (!(test_bit(BATADV_ORIG_CAPA_HAS_NC, &orig_node->capabilities))) goto out;
/* accept ogms from 'good' neighbors and single hop neighbors */ diff --git a/types.h b/types.h index 08a6343..cb7ccb1 100644 --- a/types.h +++ b/types.h @@ -300,7 +300,7 @@ struct batadv_orig_node { */ enum batadv_orig_capabilities { BATADV_ORIG_CAPA_HAS_DAT, - BATADV_ORIG_CAPA_HAS_NC = BIT(1), + BATADV_ORIG_CAPA_HAS_NC, BATADV_ORIG_CAPA_HAS_TT = BIT(2), BATADV_ORIG_CAPA_HAS_MCAST = BIT(3), };
On Friday, July 03, 2015 18:29:57 Linus Lüssing wrote:
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: 586df9e2537b ("batman-adv: Make NC capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
network-coding.c | 2 +- types.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Applied in revision 188ac4f.
Thanks, Marek
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: a51fa16ecf3f ("batman-adv: Make TT capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- translation-table.c | 3 ++- types.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/translation-table.c b/translation-table.c index b6c0f52..d73b103 100644 --- a/translation-table.c +++ b/translation-table.c @@ -3323,7 +3323,8 @@ static void batadv_tt_update_orig(struct batadv_priv *bat_priv, bool has_tt_init;
tt_vlan = (struct batadv_tvlv_tt_vlan_data *)tt_buff; - has_tt_init = orig_node->capa_initialized & BATADV_ORIG_CAPA_HAS_TT; + has_tt_init = test_bit(BATADV_ORIG_CAPA_HAS_TT, + &orig_node->capa_initialized);
/* orig table not initialised AND first diff is in the OGM OR the ttvn * increased by one -> we can apply the attached changes diff --git a/types.h b/types.h index cb7ccb1..6f4486c 100644 --- a/types.h +++ b/types.h @@ -301,7 +301,7 @@ struct batadv_orig_node { enum batadv_orig_capabilities { BATADV_ORIG_CAPA_HAS_DAT, BATADV_ORIG_CAPA_HAS_NC, - BATADV_ORIG_CAPA_HAS_TT = BIT(2), + BATADV_ORIG_CAPA_HAS_TT, BATADV_ORIG_CAPA_HAS_MCAST = BIT(3), };
On Friday, July 03, 2015 18:29:58 Linus Lüssing wrote:
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: a51fa16ecf3f ("batman-adv: Make TT capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
translation-table.c | 3 ++- types.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
Applied in revision a4824ff.
Thanks, Marek
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: 201a54ba710a ("batman-adv: Make MCAST capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- multicast.c | 11 ++++++----- types.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/multicast.c b/multicast.c index b75bcc3..ee8317f 100644 --- a/multicast.c +++ b/multicast.c @@ -709,14 +709,15 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv, mcast_flags = *(uint8_t *)tvlv_value;
spin_lock_bh(&orig->mcast_handler_lock); - orig_initialized = orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST; + orig_initialized = test_bit(BATADV_ORIG_CAPA_HAS_MCAST, + &orig->capa_initialized);
/* If mcast support is turned on decrease the disabled mcast node * counter only if we had increased it for this node before. If this * is a completely new orig_node no need to decrease the counter. */ if (orig_mcast_enabled && - !(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST)) { + !(test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities))) { if (orig_initialized) atomic_dec(&bat_priv->mcast.num_disabled); set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities); @@ -725,7 +726,7 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv, * node counter. */ } else if (!orig_mcast_enabled && - (orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST || + (test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) || !orig_initialized)) { atomic_inc(&bat_priv->mcast.num_disabled); clear_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities); @@ -774,8 +775,8 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
spin_lock_bh(&orig->mcast_handler_lock);
- if (!(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST) && - orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST) + if (!(test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities)) && + test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized)) atomic_dec(&bat_priv->mcast.num_disabled);
batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS); diff --git a/types.h b/types.h index 6f4486c..3baf595 100644 --- a/types.h +++ b/types.h @@ -302,7 +302,7 @@ enum batadv_orig_capabilities { BATADV_ORIG_CAPA_HAS_DAT, BATADV_ORIG_CAPA_HAS_NC, BATADV_ORIG_CAPA_HAS_TT, - BATADV_ORIG_CAPA_HAS_MCAST = BIT(3), + BATADV_ORIG_CAPA_HAS_MCAST, };
/**
On Friday, July 03, 2015 18:29:59 Linus Lüssing wrote:
The introduction of set_bit() and clear_bit() calls in batman-adv wrongly passed bitmasks and not the bit numbers to these functions. This leads to broken capability checks.
Fixing this by making the capability enum a non-bitmasked one and by that passing non-masked values to set_bit()/clear_bit().
Fixes: 201a54ba710a ("batman-adv: Make MCAST capability changes atomic") Reported-by: Def def@laposte.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
multicast.c | 11 ++++++----- types.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-)
Applied in revision 1798ad3.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Linus Lüssing -
Marek Lindner