netdev - Re: [PATCH 2/2] net: Introduce sk_clone

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 3 Mar 2017 12:34:43 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        "David S . Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, Arnaldo Carvalho de Melo <acme@...hat.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Gerrit Renker <gerrit@....abdn.ac.uk>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 2/2] net: Introduce sk_clone_lock() error path routine

Hello!

On 3/1/2017 10:35 PM, Arnaldo Carvalho de Melo wrote:

> From: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> When handling problems in cloning a socket with the sk_clone_locked()
> function we need to perform several steps that were open coded in it and
> its callers, so introduce a routine to avoid this duplication:
> sk_free_unlock_clone().
>
> Cc: Cong Wang <xiyou.wangcong@...il.com>
> Cc: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Gerrit Renker <gerrit@....abdn.ac.uk>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Link: http://lkml.kernel.org/n/net-ui6laqkotycunhtmqryl9bfx@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
[...]
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 4eca27dc5c94..a3d9bb20f65d 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1540,11 +1540,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
>  			is_charged = sk_filter_charge(newsk, filter);
>
>  		if (unlikely(!is_charged || xfrm_sk_clone_policy(newsk, sk))) {
> -			/* It is still raw copy of parent, so invalidate
> -			 * destructor and make plain sk_free() */
> -			newsk->sk_destruct = NULL;
> -			bh_unlock_sock(newsk);
> -			sk_free(newsk);
> +			sk_free_unlock_clone(newsk);
>  			newsk = NULL;
>  			goto out;
>  		}
> @@ -1593,6 +1589,16 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
>  }
>  EXPORT_SYMBOL_GPL(sk_clone_lock);
>
> +void sk_free_unlock_clone(struct sock *sk)
> +{
> +	/* It is still raw copy of parent, so invalidate
> +	 * destructor and make plain sk_free() */

    Could fix the comment style to the preferred one, while at it:

/* bla
  * bla
  */

> +	sk->sk_destruct = NULL;
> +	bh_unlock_sock(sk);
> +	sk_free(sk);
> +}
> +EXPORT_SYMBOL_GPL(sk_free_unlock_clone);
> +
>  void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
>  {
>  	u32 max_segs = 1;
[...]

MBR, Sergei