Re: [PATCH RESEND 1/2] neigh: only allow init_net to change the default neigh_parms

[ebiederm@xmission.com (Eric W. Biederman)]

Gao feng <gaofeng@cn.fujitsu.com> writes:

> On 06/12/2013 03:33 PM, Eric W. Biederman wrote:
>> Gao feng <gaofeng@cn.fujitsu.com> writes:
>> 
>>> Though we don't export the /proc/sys/net/ipv[4,6]/neigh/default/
>>> directory to the un-init_net, but we can still use cmd such as
>>> "ip ntable change name arp_cache locktime 129" to change the locktime
>>> of default neigh_parms.
>>>
>>> This patch disallows the un-init_net to find out the neigh_table.parms.
>>> So the un-init_net will failed to influence the init_net.
>> 
>> Interesting...  
>> 
>> The problem these two patches seek to address seems legit.
>> 
>> However I disagree with the way you are handling this.
>> 
>> Outside of the initial network namespace we should return -ENOENT
>> instead of -EPERM.  Which would match how we handle sysctls, and I think
>> missing neigh table values.  Just not making these global values visible
>> seems wise.
>> 
>
> Ok, it seems more reasonable.
>
>> The alternative is to use the proper permission test which is
>> capable(CAP_SYS_ADMIN) (instead of testing network namespaces) and
>> return -EPERM if that fails.  Which would allow processes in other
>> network namespaces to change the value if they could otherwise change
>> the value.
>> 
>
> So you mean the uninitial net namespace can't see these values but it
> can change them? it's too strange.

Sorry I was saying that if you don't want to hide the values the
permissions and (-EPERM) should track the user namespace not the network
namespace.

> And the thresh/interval are both under default/ too, if we return -ENOENT
> for other items, we should also return -ENOENT for them instead of the
> -EPERM.

Yes.  Let's return hide the global values and just return -ENOENT for
everything.  That seems simplest.

Eric