b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order
@ 2014-02-11 16:05 Antonio Quartulli
  2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Antonio Quartulli @ 2014-02-11 16:05 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Antonio Quartulli

From: Antonio Quartulli <antonio@open-mesh.com>

When computing the CRC on a 2byte variable the order of
the bytes obviously alters the final result. This means
that computing the CRC over the same value on two archs
having different endianess leads to different numbers.

The global and local translation table CRC computation
routine makes this mistake while processing the clients
VIDs. The result is a continuous CRC mismatching between
nodes having different endianess.

Fix this by converting the VID to Network Order before
processing it. This guarantees that every node uses the same
byte order.

Introduced by 21a57f6e7a3b4455dfe68ee07a7b901d9e7f200b
("batman-adv: make the TT CRC logic VLAN specific")

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 translation-table.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/translation-table.c b/translation-table.c
index 05c2a9b..24e3267 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1961,6 +1961,7 @@ static uint32_t batadv_tt_global_crc(struct batadv_priv *bat_priv,
 	struct hlist_head *head;
 	uint32_t i, crc_tmp, crc = 0;
 	uint8_t flags;
+	__be16 tmp_vid;
 
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
@@ -1997,8 +1998,11 @@ static uint32_t batadv_tt_global_crc(struct batadv_priv *bat_priv,
 							     orig_node))
 				continue;
 
-			crc_tmp = crc32c(0, &tt_common->vid,
-					 sizeof(tt_common->vid));
+			/* use network order to read the VID: this ensures that
+			 * every node reads the bytes in the same order.
+			 */
+			tmp_vid = htons(tt_common->vid);
+			crc_tmp = crc32c(0, &tmp_vid, sizeof(tmp_vid));
 
 			/* compute the CRC on flags that have to be kept in sync
 			 * among nodes
@@ -2032,6 +2036,7 @@ static uint32_t batadv_tt_local_crc(struct batadv_priv *bat_priv,
 	struct hlist_head *head;
 	uint32_t i, crc_tmp, crc = 0;
 	uint8_t flags;
+	__be16 tmp_vid;
 
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
@@ -2050,8 +2055,11 @@ static uint32_t batadv_tt_local_crc(struct batadv_priv *bat_priv,
 			if (tt_common->flags & BATADV_TT_CLIENT_NEW)
 				continue;
 
-			crc_tmp = crc32c(0, &tt_common->vid,
-					 sizeof(tt_common->vid));
+			/* use network order to read the VID: this ensures that
+			 * every node reads the bytes in the same order.
+			 */
+			tmp_vid = htons(tt_common->vid);
+			crc_tmp = crc32c(0, &tmp_vid, sizeof(tmp_vid));
 
 			/* compute the CRC on flags that have to be kept in sync
 			 * among nodes
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success
  2014-02-11 16:05 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
@ 2014-02-11 16:05 ` Antonio Quartulli
  2014-02-11 17:13   ` Russell Senior
  2014-02-15  0:36   ` Marek Lindner
  2014-02-11 16:56 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 7+ messages in thread
From: Antonio Quartulli @ 2014-02-11 16:05 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Antonio Quartulli

From: Antonio Quartulli <antonio@open-mesh.com>

When the TVLV parsing routine succeed the skb is left
untouched thus leading to a memory leak.

Fix this by consuming the skb in case of success.

Introduced by 0b6aa0d43767889eeda43a132cf5e73df4e63bf2
("batman-adv: tvlv - basic infrastructure")

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 routing.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/routing.c b/routing.c
index f7579d0..71bf698 100644
--- a/routing.c
+++ b/routing.c
@@ -1063,6 +1063,8 @@ int batadv_recv_unicast_tvlv(struct sk_buff *skb,
 
 	if (ret != NET_RX_SUCCESS)
 		ret = batadv_route_unicast_packet(skb, recv_if);
+	else
+		consume_skb(skb);
 
 	return ret;
 }
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order
  2014-02-11 16:05 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
  2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
@ 2014-02-11 16:56 ` Antonio Quartulli
  2014-02-11 17:31 ` Russell Senior
  2014-02-15  0:29 ` Marek Lindner
  3 siblings, 0 replies; 7+ messages in thread
From: Antonio Quartulli @ 2014-02-11 16:56 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On 11/02/14 17:05, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> When computing the CRC on a 2byte variable the order of
> the bytes obviously alters the final result. This means
> that computing the CRC over the same value on two archs
> having different endianess leads to different numbers.
> 
> The global and local translation table CRC computation
> routine makes this mistake while processing the clients
> VIDs. The result is a continuous CRC mismatching between
> nodes having different endianess.
> 
> Fix this by converting the VID to Network Order before
> processing it. This guarantees that every node uses the same
> byte order.
> 
> Introduced by 21a57f6e7a3b4455dfe68ee07a7b901d9e7f200b
> ("batman-adv: make the TT CRC logic VLAN specific")
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>

Both the patch are:

Reported-by: Russel Senior <russell@personaltelco.net>


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success
  2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
@ 2014-02-11 17:13   ` Russell Senior
  2014-02-15  0:36   ` Marek Lindner
  1 sibling, 0 replies; 7+ messages in thread
From: Russell Senior @ 2014-02-11 17:13 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking


From: Antonio Quartulli <antonio@open-mesh.com>
When the TVLV parsing routine succeed the skb is left
untouched thus leading to a memory leak.

Fix this by consuming the skb in case of success.

Introduced by 0b6aa0d43767889eeda43a132cf5e73df4e63bf2
("batman-adv: tvlv - basic infrastructure")

Reported-by: Russell Senior <russell@personaltelco.net>
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
Tested-by: Russell Senior <russell@personaltelco.net>
---
 routing.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/routing.c b/routing.c
index f7579d0..71bf698 100644
--- a/routing.c
+++ b/routing.c
@@ -1063,6 +1063,8 @@ int batadv_recv_unicast_tvlv(struct sk_buff *skb,
 
 	if (ret != NET_RX_SUCCESS)
 		ret = batadv_route_unicast_packet(skb, recv_if);
+	else
+		consume_skb(skb);
 
 	return ret;
 }
-- 
1.8.5.3




-- 
Russell Senior, President
russell@personaltelco.net

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order
  2014-02-11 16:05 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
  2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
  2014-02-11 16:56 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
@ 2014-02-11 17:31 ` Russell Senior
  2014-02-15  0:29 ` Marek Lindner
  3 siblings, 0 replies; 7+ messages in thread
From: Russell Senior @ 2014-02-11 17:31 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking
  Cc: Antonio Quartulli


Tested-by: Russell Senior <russell@personaltelco.net>

-- 
Russell Senior, President
russell@personaltelco.net

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order
  2014-02-11 16:05 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
                   ` (2 preceding siblings ...)
  2014-02-11 17:31 ` Russell Senior
@ 2014-02-15  0:29 ` Marek Lindner
  3 siblings, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2014-02-15  0:29 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Antonio Quartulli, Antonio Quartulli

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Tuesday 11 February 2014 17:05:06 Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> When computing the CRC on a 2byte variable the order of
> the bytes obviously alters the final result. This means
> that computing the CRC over the same value on two archs
> having different endianess leads to different numbers.
> 
> The global and local translation table CRC computation
> routine makes this mistake while processing the clients
> VIDs. The result is a continuous CRC mismatching between
> nodes having different endianess.
> 
> Fix this by converting the VID to Network Order before
> processing it. This guarantees that every node uses the same
> byte order.
> 
> Introduced by 21a57f6e7a3b4455dfe68ee07a7b901d9e7f200b
> ("batman-adv: make the TT CRC logic VLAN specific")
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  translation-table.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)

Applied in revision be4385e.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success
  2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
  2014-02-11 17:13   ` Russell Senior
@ 2014-02-15  0:36   ` Marek Lindner
  1 sibling, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2014-02-15  0:36 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Antonio Quartulli, Antonio Quartulli

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

On Tuesday 11 February 2014 17:05:07 Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> When the TVLV parsing routine succeed the skb is left
> untouched thus leading to a memory leak.
> 
> Fix this by consuming the skb in case of success.
> 
> Introduced by 0b6aa0d43767889eeda43a132cf5e73df4e63bf2
> ("batman-adv: tvlv - basic infrastructure")
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  routing.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied in revision 9289542.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-02-15  0:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-11 16:05 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
2014-02-11 16:05 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: free skb on TVLV parsing success Antonio Quartulli
2014-02-11 17:13   ` Russell Senior
2014-02-15  0:36   ` Marek Lindner
2014-02-11 16:56 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: fix TT CRC computation by ensuring byte order Antonio Quartulli
2014-02-11 17:31 ` Russell Senior
2014-02-15  0:29 ` Marek Lindner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).