Skip to contentSkip to conversation

Possible bug in net/core/neighbor.c

DISCUSSION Ulf Samuelsson 1 participant 2 messages Nov 5, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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.

Signature · 2 lines
Best Regards,
Ulf Samuelsson
#1 lore raw
Quoted 38 lines · Ulf Samuelsson

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 
Quoted 5 lines

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
[email protected]
+46 722 427 437