Hi David

While browsing net/ipv4/route.c  I discovered compare_keys() function, and a 
potential bug in it.

static inline int compare_keys(struct flowi *fl1, struct flowi *fl2)
{
        return memcmp(&fl1->nl_u.ip4_u, &fl2->nl_u.ip4_u, 
sizeof(fl1->nl_u.ip4_u)) == 0 &&
               fl1->oif     == fl2->oif &&
               fl1->iif     == fl2->iif;
}


Using memcmp(ptr1, ptr2, sizeof(SOMEFIELD)) is dangerous because 
sizeof(SOMEFIELD) can be larger than the underlying object, because of 
alignment constraints.

In this case, sizeof(fl1->nl_u.ip4_u) is 16, while fl1->nl_u.ip4_u is :

                struct {
                        __u32                   daddr;
                        __u32                   saddr;
                        __u32                   fwmark;
                        __u8                    tos;
                        __u8                    scope;
                } ip4_u;

So 14 bytes are really initialized, and 2 padding bytes might contain random 
values...

So at the very minimum, we should avoid doing the memcmp() including those 
last two bytes : It would be less bugy, and faster too...  (But to get really 
fast comparison, we should do some clever long/int XOR operations to avoid 
many test/branches, like the optim we did in compare_ether_addr())

As shown in profiles, "repz cmpsb" is really a dog... (and its use of 
esi/edi/ecx registers a high pressure for the compiler/optimizer)

Eric

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

Reply via email to