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

Marek Lindner
2018-05-09T14:08:56Z

[-- 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 --]