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
pgppdgx55t7U4.pgp
Description: PGP signature
