Subject: Re: [PATCH] net: Fix IP_MULTICAST_IF

From: Eric Dumazet <[email-masked]>
Date: Tue, 20 Oct 2009 06:07:48 +0200

David Miller a écrit :
> From: Eric Dumazet <[email-masked]>
> Date: Mon, 19 Oct 2009 18:41:58 +0200
> 
>> ipv4/ipv6 setsockopt(IP_MULTICAST_IF) have dubious __dev_get_by_index() calls.
> 
> Dubious, how so?
> 
> Yes, I know RTNL/dev_base_lock, but it's not using what it gets
> back at all.
> 
> It's testing existence, a boolean, it doesn't dereference the
> 'dev' it gets back at all.
> 
> This code is intentional and perfectly fine.

If this was intentional, something changed and made the prereq false.

Final target might be fine, but an element in the chain, before target
could be deleted while reader scans hash chain.

/* Device list removal */
static void unlist_netdevice(struct net_device *dev)
{
        ASSERT_RTNL();

        /* Unlink dev from the device chain */
        write_lock_bh(&dev_base_lock);
        list_del(&dev->dev_list);
        hlist_del(&dev->name_hlist);
        hlist_del(&dev->index_hlist);   <<< HERE >>>
        write_unlock_bh(&dev_base_lock);
}

static inline void hlist_del(struct hlist_node *n)
{
        __hlist_del(n);
        n->next = LIST_POISON1;   <<< HERE >>>
        n->pprev = LIST_POISON2;
}
include/linux/poison.h:#define LIST_POISON1  ((void *) 0x00100100)

reader tries to pass over this delete net_device, doing a dev->index_hlist->next

#define hlist_for_each(pos, head) \
        for (pos = (head)->first; pos && ({ prefetch(pos->next); 1; }); \
             pos = pos->next)

So it should visit a nice memory location ?