From: Vitaliy Makkoveev <otto@bsdbox.dev>
Subject: Re: Remove superfluous check from sysctl(2)
To: Christian Ludwig <cludwig@genua.de>
Cc: tech@openbsd.org
Date: Sun, 7 Jul 2024 20:45:45 +0300

Please no. I have the diff which pushes kernel lock down to
sys_sysctl(), so this logic will come back.

> On 7 Jul 2024, at 19:50, Christian Ludwig <cludwig@genua.de> wrote:
> 
> Hi,
> 
> The last user that has set dolock = 0 was removed in v1.272, 2014/11/19.
> I think it's safe to remove that variable.
> 
> 
> Have fun.
> 
> ---
> sys/kern/kern_sysctl.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
> index 5d0d1a8a851..6606c34a187 100644
> --- a/sys/kern/kern_sysctl.c
> +++ b/sys/kern/kern_sysctl.c
> @@ -172,7 +172,7 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
> 		syscallarg(void *) new;
> 		syscallarg(size_t) newlen;
> 	} */ *uap = v;
> -	int error, dolock = 1;
> +	int error;
> 	size_t savelen = 0, oldlen = 0;
> 	sysctlfn *fn;
> 	int name[CTL_MAXNAME];
> @@ -237,25 +237,22 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
> 	if (SCARG(uap, old) != NULL) {
> 		if ((error = rw_enter(&sysctl_lock, RW_WRITE|RW_INTR)) != 0)
> 			return (error);
> -		if (dolock) {
> -			if (atop(oldlen) > uvmexp.wiredmax - uvmexp.wired) {
> -				rw_exit_write(&sysctl_lock);
> -				return (ENOMEM);
> -			}
> -			error = uvm_vslock(p, SCARG(uap, old), oldlen,
> -			    PROT_READ | PROT_WRITE);
> -			if (error) {
> -				rw_exit_write(&sysctl_lock);
> -				return (error);
> -			}
> +		if (atop(oldlen) > uvmexp.wiredmax - uvmexp.wired) {
> +			rw_exit_write(&sysctl_lock);
> +			return (ENOMEM);
> +		}
> +		error = uvm_vslock(p, SCARG(uap, old), oldlen,
> +		    PROT_READ | PROT_WRITE);
> +		if (error) {
> +			rw_exit_write(&sysctl_lock);
> +			return (error);
> 		}
> 		savelen = oldlen;
> 	}
> 	error = (*fn)(&name[1], SCARG(uap, namelen) - 1, SCARG(uap, old),
> 	    &oldlen, SCARG(uap, new), SCARG(uap, newlen), p);
> 	if (SCARG(uap, old) != NULL) {
> -		if (dolock)
> -			uvm_vsunlock(p, SCARG(uap, old), savelen);
> +		uvm_vsunlock(p, SCARG(uap, old), savelen);
> 		rw_exit_write(&sysctl_lock);
> 	}
> 	if (error)
> -- 
> 2.34.1
>