netdev - Re: [PATCH next] arp: avoid sending ucast probes to 00:00:00:00:00:00

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Dec 2016 15:47:32 -0800
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH next] arp: avoid sending ucast probes to 00:00:00:00:00:00

[...]
>> @@ -371,10 +372,12 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
>>
>>       probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
>>       if (probes < 0) {
>> +             memset(&null_dev_hw_addr, 0, dev->addr_len);
>>               if (!(neigh->nud_state & NUD_VALID))
>>                       pr_debug("trying to ucast probe in NUD_INVALID\n");
>>               neigh_ha_snapshot(dst_ha, neigh, dev);
>> -             dst_hw = dst_ha;
>> +             if (memcmp(&dst_ha, &null_dev_hw_addr, dev->addr_len) != 0)
>> +                     dst_hw = dst_ha;
>>       } else {
>>               probes -= NEIGH_VAR(neigh->parms, APP_PROBES);
>>               if (probes < 0) {
>
> Why is is an IPv4 specific issue ?
I think the issue is that neigh_ha_snapshot() gets neigh->ha
unconditionally even if the neigh state is NUD_INVALID.

> What about IPv6 ?
Well it's not ARP. The ndisc_solicit() calls ndisc_send_ns() with
neigh parameter for unicast probe while call with NULL for the
broadcast probe case. However it does not use this parameter in
unicast case and probably relies on the route-entry. Hence it is not
subjected to the same issue.
>
>
> I would try something in neighbour code, maybe :
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 782dd866366554e53dda3e6c69c807ec90bd0e08..fdfb177eecb6a9b1479eedde457cb1f652d32c68 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -916,7 +916,10 @@ static void neigh_timer_handler(unsigned long arg)
>                         neigh_dbg(2, "neigh %p is probed\n", neigh);
>                         neigh->nud_state = NUD_PROBE;
>                         neigh->updated = jiffies;
> -                       atomic_set(&neigh->probes, 0);
> +                       atomic_set(&neigh->probes,
> +                                  (neigh->output == neigh_blackhole) ?
> +                                       NEIGH_VAR(neigh->parms, UCAST_PROBES) :
> +                                       0);
This would work if we change the above line (in arp_solicit() code)
from 'if (probes < 0)' to 'if (probes <= 0)'.

>                         notify = 1;
>                         next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
>                 }
>
> Thanks.
>
>