netdev - Re: [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d3a47cb1-a320-cc2f-d0ea-afa64c590d33@virtuozzo.com>
Date:   Fri, 28 Feb 2020 17:43:49 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next v2 1/2] unix: uses an atomic type for scm files
 accounting

On 28.02.2020 16:45, Paolo Abeni wrote:
> So the scm_stat_{add,del} helper can be invoked with no
> additional lock held.
> 
> This clean-up the code a bit and will make the next
> patch easier.
> 
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>

Reviewed-by: Kirill Tkhai <ktkhai@...tuozzo.com>

> ---
>  include/net/af_unix.h |  2 +-
>  net/unix/af_unix.c    | 21 ++++++---------------
>  2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 17e10fba2152..5cb65227b7a9 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -42,7 +42,7 @@ struct unix_skb_parms {
>  } __randomize_layout;
>  
>  struct scm_stat {
> -	u32 nr_fds;
> +	atomic_t nr_fds;
>  };
>  
>  #define UNIXCB(skb)	(*(struct unix_skb_parms *)&((skb)->cb))
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index cbd7dc01e147..145a3965341e 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -689,7 +689,8 @@ static void unix_show_fdinfo(struct seq_file *m, struct socket *sock)
>  
>  	if (sk) {
>  		u = unix_sk(sock->sk);
> -		seq_printf(m, "scm_fds: %u\n", READ_ONCE(u->scm_stat.nr_fds));
> +		seq_printf(m, "scm_fds: %u\n",
> +			   atomic_read(&u->scm_stat.nr_fds));
>  	}
>  }
>  
> @@ -1598,10 +1599,8 @@ static void scm_stat_add(struct sock *sk, struct sk_buff *skb)
>  	struct scm_fp_list *fp = UNIXCB(skb).fp;
>  	struct unix_sock *u = unix_sk(sk);
>  
> -	lockdep_assert_held(&sk->sk_receive_queue.lock);
> -
>  	if (unlikely(fp && fp->count))
> -		u->scm_stat.nr_fds += fp->count;
> +		atomic_add(fp->count, &u->scm_stat.nr_fds);
>  }
>  
>  static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
> @@ -1609,10 +1608,8 @@ static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
>  	struct scm_fp_list *fp = UNIXCB(skb).fp;
>  	struct unix_sock *u = unix_sk(sk);
>  
> -	lockdep_assert_held(&sk->sk_receive_queue.lock);
> -
>  	if (unlikely(fp && fp->count))
> -		u->scm_stat.nr_fds -= fp->count;
> +		atomic_sub(fp->count, &u->scm_stat.nr_fds);
>  }
>  
>  /*
> @@ -1801,10 +1798,8 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
>  	if (sock_flag(other, SOCK_RCVTSTAMP))
>  		__net_timestamp(skb);
>  	maybe_add_creds(skb, sock, other);
> -	spin_lock(&other->sk_receive_queue.lock);
>  	scm_stat_add(other, skb);
> -	__skb_queue_tail(&other->sk_receive_queue, skb);
> -	spin_unlock(&other->sk_receive_queue.lock);
> +	skb_queue_tail(&other->sk_receive_queue, skb);
>  	unix_state_unlock(other);
>  	other->sk_data_ready(other);
>  	sock_put(other);
> @@ -1906,10 +1901,8 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>  			goto pipe_err_free;
>  
>  		maybe_add_creds(skb, sock, other);
> -		spin_lock(&other->sk_receive_queue.lock);
>  		scm_stat_add(other, skb);
> -		__skb_queue_tail(&other->sk_receive_queue, skb);
> -		spin_unlock(&other->sk_receive_queue.lock);
> +		skb_queue_tail(&other->sk_receive_queue, skb);
>  		unix_state_unlock(other);
>  		other->sk_data_ready(other);
>  		sent += size;
> @@ -2405,9 +2398,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>  			sk_peek_offset_bwd(sk, chunk);
>  
>  			if (UNIXCB(skb).fp) {
> -				spin_lock(&sk->sk_receive_queue.lock);
>  				scm_stat_del(sk, skb);
> -				spin_unlock(&sk->sk_receive_queue.lock);
>  				unix_detach_fds(&scm, skb);
>  			}
>  
>