batman-adv: tvlv: avoid race of cifsnotfound handler state

TVLV handlers can have the flag BATADV_TVLV_HANDLER_OGM_CIFNOTFND set to
signal that the OGM handler should be called (with NULL for data) when the
specific TVLV container was not found in the OGM. This is used by:

* DAT
* GW
* Multicast (OGM + Tracker)

The state whether the handler was executed was stored in the struct
batadv_tvlv_handler. But the TVLV processing is started without any lock.
Multiple parallel contexts processing TVLVs would therefore overwrite each
others BATADV_TVLV_HANDLER_OGM_CALLED flag in the shared
batadv_tvlv_handler.

Drop the shared BATADV_TVLV_HANDLER_OGM_CALLED flag and instead determine,
per TVLV buffer, whether a matching container was present by scanning the
packet's buffer.

Cc: stable@kernel.org
Fixes: ef26157747d4 ("batman-adv: tvlv - basic infrastructure")
Signed-off-by: Sven Eckelmann <sven@narfation.org>

diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c
index a957555d8958d1be83be86d971f004fa03a1d42e..1c9fb21985f6a558de1a1fc9ba57ab48abccbb7c 100644 (file)
--- a/net/batman-adv/tvlv.c
+++ b/net/batman-adv/tvlv.c
@@ -411,7 +411,6 @@ static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv,
                tvlv_handler->ogm_handler(bat_priv, orig_node,
                                          BATADV_NO_FLAGS,
                                          tvlv_value, tvlv_value_len);
-               tvlv_handler->flags |= BATADV_TVLV_HANDLER_OGM_CALLED;
                break;
        case BATADV_UNICAST_TVLV:
                if (!skb)
@@ -443,6 +442,48 @@ static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv,
        return NET_RX_SUCCESS;
 }
 
+/**
+ * batadv_tvlv_containers_contain() - check if a tvlv buffer holds a container
+ * @tvlv_value: tvlv content
+ * @tvlv_value_len: tvlv content length
+ * @type: tvlv container type to look for
+ * @version: tvlv container version to look for
+ *
+ * Return: true if a container of the given type and version is present in the
+ * tvlv buffer, false otherwise.
+ */
+static bool batadv_tvlv_containers_contain(void *tvlv_value,
+                                          u16 tvlv_value_len, u8 type,
+                                          u8 version)
+{
+       struct batadv_tvlv_hdr *tvlv_hdr;
+       u16 tvlv_value_cont_len;
+
+       while (tvlv_value_len >= sizeof(*tvlv_hdr)) {
+               tvlv_hdr = tvlv_value;
+               tvlv_value_cont_len = ntohs(tvlv_hdr->len);
+               tvlv_value = tvlv_hdr + 1;
+               tvlv_value_len -= sizeof(*tvlv_hdr);
+
+               if (tvlv_value_cont_len > tvlv_value_len)
+                       break;
+
+               /* the next tvlv header is accessed assuming (at least) 2-byte
+                * alignment, so it must start at an even offset.
+                */
+               if (tvlv_value_cont_len & 1)
+                       break;
+
+               if (tvlv_hdr->type == type && tvlv_hdr->version == version)
+                       return true;
+
+               tvlv_value = (u8 *)tvlv_value + tvlv_value_cont_len;
+               tvlv_value_len -= tvlv_value_cont_len;
+       }
+
+       return false;
+}
+
 /**
  * batadv_tvlv_containers_process() - parse the given tvlv buffer to call the
  *  appropriate handlers
@@ -462,7 +503,9 @@ int batadv_tvlv_containers_process(struct batadv_priv *bat_priv,
                                   struct sk_buff *skb, void *tvlv_value,
                                   u16 tvlv_value_len)
 {
+       u16 tvlv_value_start_len = tvlv_value_len;
        struct batadv_tvlv_handler *tvlv_handler;
+       void *tvlv_value_start = tvlv_value;
        struct batadv_tvlv_hdr *tvlv_hdr;
        u16 tvlv_value_cont_len;
        u8 cifnotfound = BATADV_TVLV_HANDLER_OGM_CIFNOTFND;
@@ -506,12 +549,20 @@ int batadv_tvlv_containers_process(struct batadv_priv *bat_priv,
                if (!tvlv_handler->ogm_handler)
                        continue;
 
-               if ((tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND) &&
-                   !(tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CALLED))
-                       tvlv_handler->ogm_handler(bat_priv, orig_node,
-                                                 cifnotfound, NULL, 0);
+               if (!(tvlv_handler->flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND))
+                       continue;
 
-               tvlv_handler->flags &= ~BATADV_TVLV_HANDLER_OGM_CALLED;
+               /* if the corresponding container was present then the handler
+                * was already called from the loop above
+                */
+               if (batadv_tvlv_containers_contain(tvlv_value_start,
+                                                  tvlv_value_start_len,
+                                                  tvlv_handler->type,
+                                                  tvlv_handler->version))
+                       continue;
+
+               tvlv_handler->ogm_handler(bat_priv, orig_node,
+                                         cifnotfound, NULL, 0);
        }
        rcu_read_unlock();
 

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 3de3c1ac0244f43b8b06f6437db1895beff2fecf..b1f9f8964c3fd21bfabe27fefd32e8a9fe975510 100644 (file)
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -2294,13 +2294,6 @@ enum batadv_tvlv_handler_flags {
         *  will call this handler even if its type was not found (with no data)
         */
        BATADV_TVLV_HANDLER_OGM_CIFNOTFND = BIT(1),
-
-       /**
-        * @BATADV_TVLV_HANDLER_OGM_CALLED: interval tvlv handling flag - the
-        *  API marks a handler as being called, so it won't be called if the
-        *  BATADV_TVLV_HANDLER_OGM_CIFNOTFND flag was set
-        */
-       BATADV_TVLV_HANDLER_OGM_CALLED = BIT(2),
 };
 
 #endif /* _NET_BATMAN_ADV_TYPES_H_ */