From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: sysctl(2): unlock fs_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 30 Oct 2024 21:32:59 +0100

On Wed, Oct 30, 2024 at 07:27:20PM +0300, Vitaliy Makkoveev wrote:
> It is the only `suid_clear' variable - atomically accessed integer.

OK bluhm@

> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.450 kern_sysctl.c
> --- sys/kern/kern_sysctl.c	28 Oct 2024 10:18:03 -0000	1.450
> +++ sys/kern/kern_sysctl.c	30 Oct 2024 16:12:02 -0000
> @@ -262,6 +262,7 @@ sys_sysctl(struct proc *p, void *v, regi
>  		fn = net_sysctl;
>  		break;
>  	case CTL_FS:
> +		dolock = 0;
>  		fn = fs_sysctl;
>  		break;
>  	case CTL_VFS:
> Index: sys/kern/vfs_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/vfs_subr.c,v
> diff -u -p -r1.324 vfs_subr.c
> --- sys/kern/vfs_subr.c	18 Oct 2024 05:52:32 -0000	1.324
> +++ sys/kern/vfs_subr.c	30 Oct 2024 16:12:02 -0000
> @@ -72,6 +72,11 @@
>  
>  #include "softraid.h"
>  
> +/*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
>  void sr_quiesce(void);
>  
>  enum vtype iftovt_tab[16] = {
> @@ -84,8 +89,8 @@ int	vttoif_tab[9] = {
>  	S_IFSOCK, S_IFIFO, S_IFMT,
>  };
>  
> -int prtactive = 0;		/* 1 => print out reclaim of active vnodes */
> -int suid_clear = 1;		/* 1 => clear SUID / SGID on owner change */
> +int prtactive = 0;	/* 1 => print out reclaim of active vnodes */
> +int suid_clear = 1;	/* [a] 1 => clear SUID / SGID on owner change */
>  
>  /*
>   * Insq/Remq for the vnode usage lists.
> Index: sys/kern/vfs_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> diff -u -p -r1.368 vfs_syscalls.c
> --- sys/kern/vfs_syscalls.c	1 Sep 2024 23:26:10 -0000	1.368
> +++ sys/kern/vfs_syscalls.c	30 Oct 2024 16:12:02 -0000
> @@ -2470,7 +2470,7 @@ dofchownat(struct proc *p, int fd, const
>  			goto out;
>  		if ((uid != -1 || gid != -1) &&
>  		    !vnoperm(vp) &&
> -		    (suser(p) || suid_clear)) {
> +		    (suser(p) || atomic_load_int(&suid_clear))) {
>  			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
>  			if (error)
>  				goto out;
> @@ -2523,7 +2523,7 @@ sys_lchown(struct proc *p, void *v, regi
>  			goto out;
>  		if ((uid != -1 || gid != -1) &&
>  		    !vnoperm(vp) &&
> -		    (suser(p) || suid_clear)) {
> +		    (suser(p) || atomic_load_int(&suid_clear))) {
>  			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
>  			if (error)
>  				goto out;
> @@ -2573,7 +2573,7 @@ sys_fchown(struct proc *p, void *v, regi
>  			goto out;
>  		if ((uid != -1 || gid != -1) &&
>  		    !vnoperm(vp) &&
> -		    (suser(p) || suid_clear)) {
> +		    (suser(p) || atomic_load_int(&suid_clear))) {
>  			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
>  			if (error)
>  				goto out;