Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: filter out invalid DAT entries

[Antonio Quartulli <ordex@autistici.org>]

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

On Thu, Jan 24, 2013 at 03:44:35PM +0100, Matthias Schiffer wrote:
> On 01/24/2013 02:47 PM, Marek Lindner wrote:
> > On Thursday, January 24, 2013 21:38:58 Antonio Quartulli wrote:
> >>
> >> I thought the same, but in batadv_arp_get_type() we have a general check
> >> that discards wrong/bogus ARP request.
> >>
> >> Here instead we are filtering "correct" ARP requests that DAT should not
> >> handle.
> > 
> > What is the difference except for the naming ? In both cases we don't want 
> > these packets to be handled by DAT. 
> > 
> > Feel free to move these extra validation checks into a separate function that 
> > gets called from batadv_arp_get_type() if you wish to emphasize the difference 
> > between the types of checks. Having all checks in the same place will help to 
> > avoid overlooking things later (as already happened).
> > 
> > Cheers,
> > Marek
> > 
> 
> In my opinion, the DAT should handle the sane one of source and
> destination if one of them is sane and the other is bogus. So I would
> maybe rather move all the checks to batadv_dat_entry_add()? There it
> will only catch the add case though, not the lookup case...

I agree with Marek: adding these new checks in a separate function is probably
better. At that point batadv_arp_get_type() will directly refuse to parse
whatever ARP packet that DAT does not like.

> 
> At least a check for ff:ff:ff:ff:ff:ff should be added to maint as soon
> as possible, as such entries were actually overwriting correct DAT
> entries on my test node (and maybe even preventing ARP resolution as the
> DAT node answered with these instead of the actual addresses).
> 

Yes, I agree.
What about modifying your patch following the comments above and resend it?


Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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