Re: [PATCH net-next] sockopt: Change getsockopt() of SO_BINDTODEVICE to return an interface name

[Brian Haley <brian.haley@hp.com>]

On 11/05/2012 09:01 PM, Brian Haley wrote:
>>>>> +    rcu_read_lock();
>>>>> +    dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
>>>>> +    if (dev)
>>>>> +        strcpy(devname, dev->name);
>>>>
>>>> This still races with the device name change, potentially providing
>>>> a name which never existed in the system, doesn't it?
>>>
>>> My only argument here is that SIOCGIFNAME has had this same code
>>> forever, and
>>> noone has ever complained about that returning a garbled name.  Even
>>> dev_get_by_name() only holds an rcu lock when doing a strncmp().
>>>
>>> We'd need to audit the whole kernel to catch all the places where we
>>> potentially
>>> look at dev->name while it could change.  Is it really worth it?
>>
>> A net device name can't be changed while the device is up, or while
>> another task holds the RTNL lock.  I think that covers almost all uses.
>> I don't know whether it's worth going out to look for exceptions, but we
>> might as well fix the cases we know about.
>
> So do you think we can fix these corner cases later and get the API
> right first?

Hi Dave,

I noticed this isn't in patchwork anymore.  Is it possible to get this 
in and then work on the other issue in a separate patch since it's not 
just this code that has this problem?

Of course I'm still not convinced that extra check is necessary (but 
I'll do it to make others happy) because if you do a setsockopt("eth0"), 
all you care is that a getsockopt() returns "eth0" - if it returns 
anything else, eth0_renamed, eth0_foo, etc you'll notice it's not the 
same and take action.  And the fact that the name can't change when UP 
means the odds are small it can happen anyways.

Thanks,

-Brian