Re: [B.A.T.M.A.N.] [RFC PATCH] batman-adv: mitigate issue when empty vlan is received

[Marek Lindner <mareklindner@neomailbox.ch>]

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

On Monday, May 7, 2018 3:55:59 AM HKT Leonardo Mörlein wrote:
> -               if (!vlan)
> -                       return false;
> +               if (!vlan) {
> +                       /* Due to a bug, some originators send TT
> +                        * announcements for empty vlans. As the receiving
> +                        * nodes will ignore those empty vlans (they do not
> +                        * add a batadv_orig_node_vlan into the transglobal
> +                        * for the originating node), crc check will fail
> +                        * here. To circumvent this issue, we skip the
> +                        * verification for the vlan if the crc is
> +                        * equal to 0x00000000.
> +                        */
> +                       if (tt_vlan_tmp->crc == 0x00000000)
> +                               continue;
> +                       else
> +                               return false;
> +               }

There are some issues with this approach:

 * As you might be aware, a CRC of 0x00000000 is not an invalid or 
uninitialized CRC. That means it is conceivable the CRC over the translation 
table entries and flags results in a CRC of 0x00000000 without any bugs being 
present. Applying this patch would lead to introducing a bug in these 
conditions.

 * The second concern is about how to deal with 'the bug' (referring to your 
patch comment above).  In your case 'the bug' results in missing TT entries 
for a given VLAN plus a CRC of 0x00000000. What if the CRC was 0x00000001 due 
to some other bug ? Then you're check would totally miss the mark and we're 
back at the beginning.
If we wish to deal with such misbehavior on a scale where we assume the code 
should be able to deal with garbage values in some or all fields a different 
approach is needed. Relying on a magic number (CRC 0x00000000) is as bad as 
the current behavior (assuming each VLAN has an entry).

Shortly, I will send a patch to prevent sending an empty VLAN around. Please 
note that this patch still is more of a band-aid. We still don't know how the 
empty VLAN came into being in the first place. Plus, that patch won't protect 
against garbage packets which certainly requires more effort.

Cheers,
Marek


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