netdev - Re: Possible bug in net/core/neighbor.c

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] [day] [month] [year] [list]

Date:	Wed, 5 Nov 2014 16:46:06 +0100
From:	Ulf Samuelsson <ulf.samuelsson@...csson.com>
To:	<netdev@...r.kernel.org>
Subject: Re: Possible bug in net/core/neighbor.c

On 11/05/2014 11:46 AM, Ulf Samuelsson wrote:
> I find the following in "net/core/neighbor.c"
>
>     /* Compare new lladdr with cached one */
>     if (!dev->addr_len) {
>         /* First case: device needs no address. */
>         lladdr = neigh->ha;
>     } else if (lladdr) {
>         /* The second case: if something is already cached
>            and a new address is proposed:
>            - compare new & old
>            - if they are different, check override flag
>          */
>
>         /* POSSIBLE BUG */
>         if ((old & NUD_VALID) &&
>             !memcmp(lladdr, neigh->ha, dev->addr_len))
>             lladdr = neigh->ha;
>         /* END POSSIBLE BUG */
>     } else {
>         /* No address is supplied; if we know something,
>            use it, otherwise discard the request.
>          */
>         err = -EINVAL;
>         if (!(old & NUD_VALID))
>             goto out;
>         lladdr = neigh->ha;
>     }
>
>     It looks to me like the code is saying
>         if the proposed address is equal to the original address,
>             set the proposed address  to the original address.
>
>     which is pretty meaningless.
>
>     Should it not be:
>
>         if ((old & NUD_VALID) &&
>             memcmp(lladdr, neigh->ha, dev->addr_len))    /* True if 
> addresses are not equal */
>             neigh->ha = lladdr;             /* Update to new address */
>
>     If not, I would appreciate an explanation what the code is doing.
>
OK, I think I figured it out.

If laddr and neigh->ha are identical, we want lladdr (which is a pointer)
to have the same value as neigh->ha so after this, you know that
laddr is identical to neigh->ha by justr comparing the pointers.

When I google, I only find other people which does not understand
the purpose of the code.

The comments are also obsolete, since "check override flag" refers
to code which has been removed.

Best Regards,
Ulf Samuelsson
KI/EAB/ILM/GF
ulf.samuelsson@...csson.com
+46 722 427 437

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html