[net-next,v3] net: split rt_genid for ipv4 and ipv6

On 2013年07月26日 02:13, Hannes Frederic Sowa wrote:
> On Thu, Jul 25, 2013 at 05:47:12PM +0800, Fan Du wrote:
>> +/* For callers who don't really care about whether it's IPv4 or IPv6 */
>> +static inline void rt_genid_bump_all(struct net *net)
>> +{
>> +	atomic_inc(&net->ipv4.rt_genid);
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +	atomic_inc(&net->ipv6.rt_genid);
>> +#endif
>
> You could get away with the ifdef if you just do
> 	rt_genid_bump_ipv4(net);
> 	rt_genid_bump_ipv6(net);

Ok, will fix this.

>
> Somewhere something does break selinux:
>
>    CC      security/selinux/hooks.o
> In file included from security/selinux/hooks.c:93:0:
> security/selinux/include/xfrm.h: In function ‘selinux_xfrm_notify_policyload’:
> security/selinux/include/xfrm.h:54:2: error: implicit declaration of function ‘rt_genid_bump’ [-Werror=implicit-function-declaration]
>    rt_genid_bump(&init_net);
>    ^
> Seems like you have overlooked the rt_genid_bump in
> security/selinux/include/xfrm.h, which should be a rt_genid_bump_all
>

Thanks for report this, my side CONFIG_SECURITY_NETWORK_XFRM is not set before. :(


> Off-topic:
> Is it correct that selinux_xfrm_notify_policyload only bumps genid for
> init_net?
>

This was introduced in ee8372dd1989287c5eedb69d44bac43f69e496f1
"xfrm: invalidate dst on policy insertion/deletion" by Nicolas.

I take a look at SELINUX xfrm part, my limited understanding SELINUX XFRM rule
should take global effect on all net name space in current implementation.



Let me know if I miss something inside it. Thanks.


> Otherwise I don't see any problems arising from this patch because of
> the rt_genid split.
>
> Greetings,
>
>    Hannes
>
>

On Fri, Jul 26, 2013 at 01:49:35PM +0800, Fan Du wrote:
> diff --git a/security/selinux/include/xfrm.h 
> b/security/selinux/include/xfrm.h
> index 65f67cb..4f72d2c 100644
> --- a/security/selinux/include/xfrm.h
> +++ b/security/selinux/include/xfrm.h
> @@ -50,8 +50,14 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 
> *sid, int ckall);
> 
>  static inline void selinux_xfrm_notify_policyload(void)
>  {
> +       struct net *net;
> +
>         atomic_inc(&flow_cache_genid);
> -       rt_genid_bump(&init_net);
> +       rtnl_lock();
> +       for_each_net(net) {
> +               rt_genid_bump_all(net);
> +       }
> +       rtnl_unlock();
>  }
>  #else
>  static inline int selinux_xfrm_enabled(void)
> 
> 
> Let me know if I miss something inside it. Thanks.

I do think it is the correct change. The locking seems correct, too. I will
excercise the code with lockdep as soon as you publish a new patch.

Greetings,

  Hannes

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

On Friday, July 26, 2013 01:49:35 PM Fan Du wrote:
> I take a look at SELINUX xfrm part, my limited understanding SELINUX XFRM
> rule should take global effect on all net name space in current
> implementation.

Yes, a SELinux policy load needs to bump the cache ID as the new SELinux 
policy could have an affect on the IPsec state (SELinux label associated with 
the SAs and SPD rules).
 
> diff --git a/security/selinux/include/xfrm.h
> b/security/selinux/include/xfrm.h index 65f67cb..4f72d2c 100644
> --- a/security/selinux/include/xfrm.h
> +++ b/security/selinux/include/xfrm.h
> @@ -50,8 +50,14 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32
> *sid, int ckall);
> 
>   static inline void selinux_xfrm_notify_policyload(void)
>   {
> +       struct net *net;
> +
>          atomic_inc(&flow_cache_genid);
> -       rt_genid_bump(&init_net);
> +       rtnl_lock();
> +       for_each_net(net) {
> +               rt_genid_bump_all(net);
> +       }
> +       rtnl_unlock();
>   }
>   #else
>   static inline int selinux_xfrm_enabled(void)