Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: sysctl(2): unlock divert{,6}_sysctl()
To:
Vitaliy Makkoveev <mvs@openbsd.org>
Cc:
tech@openbsd.org
Date:
Thu, 15 Aug 2024 16:42:06 +0200

Download raw body.

Thread
On Thu, Aug 15, 2024 at 05:22:12PM +0300, Vitaliy Makkoveev wrote:
> Introduce PR_MPSYSCTL flag to mark mp-safe (*pr_sysctl)() handlers and
> unlock both divert_sysctl() and divert6_sysctl(). Unlock them together,
> because they are identical and pretty simple:
> 
> - DIVERTCTL_RECVSPACE and DIVERTCTL_SENDSPACE - atomically accessed
>   integers;
> - DIVERTCTL_STATS - per-CPU counters;

OK bluhm@

> Index: sys/kern/uipc_domain.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_domain.c,v
> diff -u -p -r1.67 uipc_domain.c
> --- sys/kern/uipc_domain.c	14 Aug 2024 17:52:47 -0000	1.67
> +++ sys/kern/uipc_domain.c	15 Aug 2024 13:21:11 -0000
> @@ -237,14 +237,18 @@ net_sysctl(int *name, u_int namelen, voi
>  	protocol = name[1];
>  	for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
>  		if (pr->pr_protocol == protocol && pr->pr_sysctl) {
> -			size_t savelen = *oldlenp;
> +			size_t savelen;
>  			int error;
>  
> -			if ((error = sysctl_vslock(oldp, savelen)))
> -				return (error);
> +			if ((pr->pr_flags & PR_MPSYSCTL) == 0) {
> +				savelen = *oldlenp;
> +				if ((error = sysctl_vslock(oldp, savelen)))
> +					return (error);
> +			}
>  			error = (*pr->pr_sysctl)(name + 2, namelen - 2,
>  			    oldp, oldlenp, newp, newlen);
> -			sysctl_vsunlock(oldp, savelen);
> +			if ((pr->pr_flags & PR_MPSYSCTL) == 0)
> +				sysctl_vsunlock(oldp, savelen);
>  
>  			return (error);
>  		}
> Index: sys/netinet/in_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_proto.c,v
> diff -u -p -r1.107 in_proto.c
> --- sys/netinet/in_proto.c	26 Jul 2024 14:38:20 -0000	1.107
> +++ sys/netinet/in_proto.c	15 Aug 2024 13:21:12 -0000
> @@ -354,7 +354,7 @@ const struct protosw inetsw[] = {
>    .pr_type	= SOCK_RAW,
>    .pr_domain	= &inetdomain,
>    .pr_protocol	= IPPROTO_DIVERT,
> -  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET,
> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL,
>    .pr_ctloutput	= rip_ctloutput,
>    .pr_usrreqs	= &divert_usrreqs,
>    .pr_init	= divert_init,
> Index: sys/netinet/ip_divert.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> diff -u -p -r1.96 ip_divert.c
> --- sys/netinet/ip_divert.c	12 Jul 2024 19:50:35 -0000	1.96
> +++ sys/netinet/ip_divert.c	15 Aug 2024 13:21:12 -0000
> @@ -41,17 +41,22 @@
>  
>  #include <net/pfvar.h>
>  
> +/*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
>  struct	inpcbtable	divbtable;
>  struct	cpumem		*divcounters;
>  
>  #ifndef DIVERT_SENDSPACE
>  #define DIVERT_SENDSPACE	(65536 + 100)
>  #endif
> -u_int   divert_sendspace = DIVERT_SENDSPACE;
> +u_int   divert_sendspace = DIVERT_SENDSPACE;	/* [a] */
>  #ifndef DIVERT_RECVSPACE
>  #define DIVERT_RECVSPACE	(65536 + 100)
>  #endif
> -u_int   divert_recvspace = DIVERT_RECVSPACE;
> +u_int   divert_recvspace = DIVERT_RECVSPACE;	/* [a] */
>  
>  #ifndef DIVERTHASHSIZE
>  #define DIVERTHASHSIZE	128
> @@ -271,7 +276,8 @@ divert_attach(struct socket *so, int pro
>  	if (error)
>  		return error;
>  
> -	error = soreserve(so, divert_sendspace, divert_recvspace);
> +	error = soreserve(so, atomic_load_int(&divert_sendspace),
> +	    atomic_load_int(&divert_recvspace));
>  	if (error)
>  		return error;
>  
> @@ -346,8 +352,6 @@ int
>  divert_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen)
>  {
> -	int error;
> -
>  	/* All sysctl names at this level are terminal. */
>  	if (namelen != 1)
>  		return (ENOTDIR);
> @@ -356,12 +360,9 @@ divert_sysctl(int *name, u_int namelen, 
>  	case DIVERTCTL_STATS:
>  		return (divert_sysctl_divstat(oldp, oldlenp, newp));
>  	default:
> -		NET_LOCK();
> -		error = sysctl_bounded_arr(divertctl_vars,
> -		    nitems(divertctl_vars), name, namelen, oldp, oldlenp, newp,
> -		    newlen);
> -		NET_UNLOCK();
> -		return (error);
> +		return (sysctl_bounded_arr(divertctl_vars,
> +		    nitems(divertctl_vars), name, namelen, oldp, oldlenp,
> +		    newp, newlen));
>  	}
>  	/* NOTREACHED */
>  }
> Index: sys/netinet6/in6_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_proto.c,v
> diff -u -p -r1.117 in6_proto.c
> --- sys/netinet6/in6_proto.c	26 Jul 2024 14:38:20 -0000	1.117
> +++ sys/netinet6/in6_proto.c	15 Aug 2024 13:21:12 -0000
> @@ -289,7 +289,7 @@ const struct protosw inet6sw[] = {
>    .pr_type	= SOCK_RAW,
>    .pr_domain	= &inet6domain,
>    .pr_protocol	= IPPROTO_DIVERT,
> -  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET,
> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL,
>    .pr_ctloutput	= rip6_ctloutput,
>    .pr_usrreqs	= &divert6_usrreqs,
>    .pr_init	= divert6_init,
> Index: sys/netinet6/ip6_divert.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
> diff -u -p -r1.96 ip6_divert.c
> --- sys/netinet6/ip6_divert.c	12 Jul 2024 19:50:35 -0000	1.96
> +++ sys/netinet6/ip6_divert.c	15 Aug 2024 13:21:12 -0000
> @@ -44,17 +44,22 @@
>  
>  #include <net/pfvar.h>
>  
> +/*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
>  struct	inpcbtable	divb6table;
>  struct	cpumem		*div6counters;
>  
>  #ifndef DIVERT_SENDSPACE
>  #define DIVERT_SENDSPACE	(65536 + 100)
>  #endif
> -u_int   divert6_sendspace = DIVERT_SENDSPACE;
> +u_int   divert6_sendspace = DIVERT_SENDSPACE;	/* [a] */
>  #ifndef DIVERT_RECVSPACE
>  #define DIVERT_RECVSPACE	(65536 + 100)
>  #endif
> -u_int   divert6_recvspace = DIVERT_RECVSPACE;
> +u_int   divert6_recvspace = DIVERT_RECVSPACE;	/* [a] */
>  
>  #ifndef DIVERTHASHSIZE
>  #define DIVERTHASHSIZE	128
> @@ -279,7 +284,8 @@ divert6_attach(struct socket *so, int pr
>  	if (error)
>  		return (error);
>  
> -	error = soreserve(so, divert6_sendspace, divert6_recvspace);
> +	error = soreserve(so, atomic_load_int(&divert6_sendspace),
> +	    atomic_load_int(&divert6_recvspace));
>  	if (error)
>  		return (error);
>  
> @@ -322,8 +328,6 @@ int
>  divert6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
>      void *newp, size_t newlen)
>  {
> -	int error;
> -
>  	/* All sysctl names at this level are terminal. */
>  	if (namelen != 1)
>  		return (ENOTDIR);
> @@ -332,12 +336,9 @@ divert6_sysctl(int *name, u_int namelen,
>  	case DIVERT6CTL_STATS:
>  		return (divert6_sysctl_div6stat(oldp, oldlenp, newp));
>  	default:
> -		NET_LOCK();
> -		error = sysctl_bounded_arr(divert6ctl_vars,
> +		return (sysctl_bounded_arr(divert6ctl_vars,
>  		    nitems(divert6ctl_vars), name, namelen, oldp, oldlenp,
> -		    newp, newlen);
> -		NET_UNLOCK();
> -		return (error);
> +		    newp, newlen));
>  	}
>  	/* NOTREACHED */
>  }
> Index: sys/sys/protosw.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/protosw.h,v
> diff -u -p -r1.67 protosw.h
> --- sys/sys/protosw.h	12 Jul 2024 19:50:35 -0000	1.67
> +++ sys/sys/protosw.h	15 Aug 2024 13:21:12 -0000
> @@ -131,6 +131,7 @@ struct protosw {
>  #define PR_SPLICE	0x0040		/* socket splicing is possible */
>  #define PR_MPINPUT	0x0080		/* input runs with shared netlock */
>  #define PR_MPSOCKET	0x0100		/* socket uses shared netlock */
> +#define PR_MPSYSCTL	0x0200		/* mp-safe sysctl(2) handler */
>  
>  /*
>   * The arguments to usrreq are: