From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: sysctl(2): push kernel lock down to net_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 14 Aug 2024 18:09:00 +0200

On Mon, Aug 12, 2024 at 09:30:59PM +0300, Vitaliy Makkoveev wrote:
> All except PF_MPLS paths are mp-safe:
> - net_link_sysctl() and following net_ifiq_sysctl() only returns
>   EOPNOTSUPP;
> - uipc_sysctl() - mp-safe atomic access to integers;
> - bpf_sysctl() - mp-safe atomic access to integers;
> - pflow_sysctl() - returns statistics from per-CPU counters;
> - pipex_sysctl() - mp-safe atomic access to integer;
> 
> Kernel lock pushed down to mpls_sysctl(). sysctl_int_bounded() do
> copying with local variable, so context switch is safe. No need to wire
> memory or take `sysctl_lock' rwlock(9).
> 
> mpls_sysctl() could be easily converted in the way of uipc_sysctl() or
> bpf_sysctl(), so if you want to test the diff please contact me.
> 
> Protocols are still locked as they was include pages wiring, so existing
> copying will not sleep. No slowdown while doing it with net lock held.

OK bluhm@

> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.437 kern_sysctl.c
> --- sys/kern/kern_sysctl.c	11 Aug 2024 15:10:53 -0000	1.437
> +++ sys/kern/kern_sysctl.c	12 Aug 2024 18:09:22 -0000
> @@ -252,6 +252,7 @@ sys_sysctl(struct proc *p, void *v, regi
>  		fn = uvm_sysctl;
>  		break;
>  	case CTL_NET:
> +		dolock = 0;
>  		fn = net_sysctl;
>  		break;
>  	case CTL_FS:
> Index: sys/kern/uipc_domain.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_domain.c,v
> diff -u -p -r1.66 uipc_domain.c
> --- sys/kern/uipc_domain.c	12 Aug 2024 11:25:27 -0000	1.66
> +++ sys/kern/uipc_domain.c	12 Aug 2024 18:09:22 -0000
> @@ -236,9 +236,18 @@ net_sysctl(int *name, u_int namelen, voi
>  		return (EISDIR);		/* overloaded */
>  	protocol = name[1];
>  	for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
> -		if (pr->pr_protocol == protocol && pr->pr_sysctl)
> -			return ((*pr->pr_sysctl)(name + 2, namelen - 2,
> -			    oldp, oldlenp, newp, newlen));
> +		if (pr->pr_protocol == protocol && pr->pr_sysctl) {
> +			size_t savelen = *oldlenp;
> +			int error;
> +
> +			if ((error = sysctl_vslock(oldp, savelen)))
> +				return (error);
> +			error = (*pr->pr_sysctl)(name + 2, namelen - 2,
> +			    oldp, oldlenp, newp, newlen);
> +			sysctl_vsunlock(oldp, savelen);
> +
> +			return (error);
> +		}
>  	return (ENOPROTOOPT);
>  }
>  
> Index: sys/netmpls/mpls_raw.c
> ===================================================================
> RCS file: /cvs/src/sys/netmpls/mpls_raw.c,v
> diff -u -p -r1.20 mpls_raw.c
> --- sys/netmpls/mpls_raw.c	29 Apr 2024 00:29:48 -0000	1.20
> +++ sys/netmpls/mpls_raw.c	12 Aug 2024 18:09:22 -0000
> @@ -58,6 +58,12 @@ int
>  mpls_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen)
>  {
> -	return sysctl_bounded_arr(mplsctl_vars, nitems(mplsctl_vars),
> +	int error;
> +
> +	KERNEL_LOCK();
> +	error = sysctl_bounded_arr(mplsctl_vars, nitems(mplsctl_vars),
>  	    name, namelen, oldp, oldlenp, newp, newlen);
> +	KERNEL_UNLOCK();
> +
> +	return error;
>  }