On Wed, May 27, 2015 at 02:32:22PM +0300, Kirill Tkhai wrote:
> hold_net() doesn't increment net refcounter if NETNS_REFCNT_DEBUG
> is not defined. In this case inet_twdr_do_twkill_work() may happen
> after network is destoyed and lead to memory corruption like this
> happened in https://jira.sw.ru/browse/PSBM-33755.
> 
> The patch replaces {hold,release}_net with {get,put}_net, which
> makes refcounters be incremented always.

In this case a netns will not be destroyed while it has time wait
sockets. It isn't expected behaviour.

I don't understand why do we need tcp_v4_kill_ve_sockets().

> 
> Such object changes, which extends its lifetime, should be checked
> for safety of refcouter waiters, who wait for 0 refcounter. I mean
> the sequence of net device shutdown for example, see
> netdev_wait_allrefs() for details.
> 
> Luckily, we don't have the same issues with struct net, because nobody
> waits for it.
> 
> PCS6 also needs the same patch.
> 
> Signed-off-by: Kirill Tkhai <[email protected]>
> ---
>  net/ipv4/inet_timewait_sock.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index e2bda07..526e3b5 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -103,7 +103,7 @@ void inet_twsk_free(struct inet_timewait_sock *tw)
>  #ifdef CONFIG_BEANCOUNTERS
>       put_beancounter(tw->tw_ub);
>  #endif
> -     release_net(twsk_net(tw));
> +     put_net(twsk_net(tw));
>       kmem_cache_free(tw->tw_prot->twsk_prot->twsk_slab, tw);
>       module_put(owner);
>  }
> @@ -202,7 +202,7 @@ struct inet_timewait_sock *inet_twsk_alloc(const struct 
> sock *sk, const int stat
>               tw->tw_ipv6only     = 0;
>               tw->tw_transparent  = inet->transparent;
>               tw->tw_prot         = sk->sk_prot_creator;
> -             twsk_net_set(tw, hold_net(sock_net(sk)));
> +             twsk_net_set(tw, get_net(sock_net(sk)));
>               /*
>                * Because we use RCU lookups, we should not set tw_refcnt
>                * to a non null value before everything is setup for this
> 
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to