netdev - Re: [PATCH v1 net-next] net: Deprecate SO_DEBUG and reclaim SOCK

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: <7e387643370020483fb53f2c1e9dfd2b9ba28818.camel@linux.ibm.com>
Date: Wed, 14 Feb 2024 09:14:33 +0100
From: Gerd Bayer <gbayer@...ux.ibm.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
        "David S. Miller"
	 <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	 <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Matthieu Baerts
	 <matttbe@...nel.org>,
        Mat Martineau <martineau@...nel.org>,
        Wenjia Zhang
	 <wenjia@...ux.ibm.com>,
        Jan Karcher <jaka@...ux.ibm.com>, Wen Gu
	 <guwen@...ux.alibaba.com>,
        Tony Lu <tonylu@...ux.alibaba.com>,
        "D . Wythe"
	 <alibuda@...ux.alibaba.com>
Cc: Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org,
        mptcp@...ts.linux.dev, linux-s390@...r.kernel.org
Subject: Re: [PATCH v1 net-next] net: Deprecate SO_DEBUG and reclaim
 SOCK_DBG bit.

On Tue, 2024-02-13 at 14:31 -0800, Kuniyuki Iwashima wrote:

Hi Kuniyuki,

I'm adding the newly appointed SMC maintainers to chime in.

> Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
> removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22
> ("net:
> remove SOCK_DEBUG macro") removed the macro.
> 
> Now is the time to deprecate the oldest socket option.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
>  include/net/sock.h  | 1 -
>  net/core/sock.c     | 6 +++---
>  net/mptcp/sockopt.c | 4 +---
>  net/smc/af_smc.c    | 5 ++---
>  4 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 88bf810394a5..0a58dc861908 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1194,10 +1194,9 @@ int sk_setsockopt(struct sock *sk, int level,
> int optname,
>  
>         switch (optname) {
>         case SO_DEBUG:
> +               /* deprecated, but kept for compatibility. */
Not 100% sure about the language - but isn't the DEBUG feature
*removed* rather than just *deprecated*?

>                 if (val && !sockopt_capable(CAP_NET_ADMIN))
>                         ret = -EACCES;
> -               else
> -                       sock_valbool_flag(sk, SOCK_DBG, valbool);
>                 break;
>         case SO_REUSEADDR:
>                 sk->sk_reuse = (valbool ? SK_CAN_REUSE :
> SK_NO_REUSE);
> @@ -1619,7 +1618,8 @@ int sk_getsockopt(struct sock *sk, int level,
> int optname,
>  
>         switch (optname) {
>         case SO_DEBUG:
> -               v.val = sock_flag(sk, SOCK_DBG);
> +               /* deprecated. */
Same here.

> +               v.val = 0;
>                 break;
>  
>         case SO_DONTROUTE:
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index da37e4541a5d..f6d90eef3d7c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct
> mptcp_sock *msk, int optname, in
>  
>                 switch (optname) {
>                 case SO_DEBUG:
> -                       sock_valbool_flag(ssk, SOCK_DBG, !!val);
> +                       /* deprecated. */
and here.

>                         break;
>                 case SO_KEEPALIVE:
>                         if (ssk->sk_prot->keepalive)
> @@ -1458,8 +1458,6 @@ static void sync_socket_options(struct
> mptcp_sock *msk, struct sock *ssk)
>                 sk_dst_reset(ssk);
>         }
>  
> -       sock_valbool_flag(ssk, SOCK_DBG, sock_flag(sk, SOCK_DBG));
> -
>         if (inet_csk(sk)->icsk_ca_ops != inet_csk(ssk)->icsk_ca_ops)
>                 tcp_set_congestion_control(ssk, msk->ca_name, false,
> true);
>         __tcp_sock_set_cork(ssk, !!msk->cork);
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 66763c74ab76..062e16a2766a 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -445,7 +445,6 @@ static int smc_bind(struct socket *sock, struct
> sockaddr *uaddr,
>                              (1UL << SOCK_LINGER) | \
>                              (1UL << SOCK_BROADCAST) | \
>                              (1UL << SOCK_TIMESTAMP) | \
> -                            (1UL << SOCK_DBG) | \
>                              (1UL << SOCK_RCVTSTAMP) | \
>                              (1UL << SOCK_RCVTSTAMPNS) | \
>                              (1UL << SOCK_LOCALROUTE) | \
> @@ -511,8 +510,8 @@ static void smc_copy_sock_settings_to_clc(struct
> smc_sock *smc)
>  
>  #define SK_FLAGS_CLC_TO_SMC ((1UL << SOCK_URGINLINE) | \
>                              (1UL << SOCK_KEEPOPEN) | \
> -                            (1UL << SOCK_LINGER) | \
> -                            (1UL << SOCK_DBG))
> +                            (1UL << SOCK_LINGER))
> +
>  /* copy only settings and flags relevant for smc from clc to smc
> socket */
>  static void smc_copy_sock_settings_to_smc(struct smc_sock *smc)
>  {
The SMC changes look good to me, so feel free to add my
Reviewed-by: Gerd Bayer <gbayer@...ux.ibm.com>