netdev - Re: [PATCH next] neigh: initialize neigh entry correctly during arp processing

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20170817003215.GQ6984@oracle.com>
Date:   Wed, 16 Aug 2017 20:32:15 -0400
From:   Sowmini Varadhan <sowmini.varadhan@...cle.com>
To:     Mahesh Bandewar <mahesh@...dewar.net>
Cc:     David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Ido Schimmel <idosch@...lanox.com>,
        Hans Liljestrand <ishkamiel@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Reshetova Elena <elena.reshetova@...el.com>,
        Florian Westphal <fw@...len.de>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        Ihar Hrachyshka <ihrachys@...hat.com>,
        David Ahern <dsa@...ulusnetworks.com>,
        Zhang Shengju <zhangshengju@...s.chinamobile.com>,
        Mahesh Bandewar <maheshb@...gle.com>
Subject: Re: [PATCH next] neigh: initialize neigh entry correctly during arp
 processing

On (08/16/17 17:02), Mahesh Bandewar wrote:
> 
> From: Mahesh Bandewar <maheshb@...gle.com>
> 
> If the ARP processing creates a neigh entry, it's immediately marked
> as STALE without timer and stays that way in that state as long as
> host do not send traffic to that neighbour.

Perhaps I dont understand the specific  packet exchange case
that your patch is trying to fix,  but if the neighbor entry
is created as a result of an incoming packet (but we have not
yet sent anything to this neighbor) then it should be marked STALE?
IOW, STALE means "Ingress path claims this adjacency, but egress
path has not been verified".  Is the problem that the neigh never
goes into PROBE?

> +	if (neigh) {
> +		if (neigh->nud_state & NUD_VALID)
> +			neigh_update(neigh, lladdr, NUD_STALE,
> +				     NEIGH_UPDATE_F_OVERRIDE, 0);
> +		else
> +			neigh_event_send(neigh, NULL);
> +	}

NUD_VALID is already a mask of 
 (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE|NUD_PROBE|NUD_STALE|NUD_DELAY)
are you sure you want to force the transition of probe/delay -> stale
here? Maybe it woudl help to describe the exact wire packet
exchange that is broken today, but fixed by your patch.

--Sowmini