On Thu, 2016-12-01 at 15:47 -0800, Mahesh Bandewar (महेश बंडेवार) wrote:
> [...]
> >> @@ -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.
Well, it looks like the issue is in neighbour code.
Fact that IPv6 might not be impacted is not the point.
> >
> >
> > 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)'.
Then code at line 973 is wrong ?
atomic_set(&neigh->probes,
NEIGH_VAR(neigh->parms, UCAST_PROBES));
That would be a more serious issue :)
