Make writes to the f_flag field of `struct file' MP-safe using atomic

operations. Since the type of f_flag must change in order to use the
atomic(9) API, reorder the struct in order to avoid padding; as pointed
out by tedu@.

ok mpi@ visa@

4 files changed, 19 insertions, 16 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 1de195a2851..9a35e7c1e13 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.196 2020/01/08 16:45:28 visa Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.197 2020/02/01 08:57:27 anton Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -410,7 +410,7 @@ sys_fcntl(struct proc *p, void *v, register_t *retval)
 	struct filedesc *fdp = p->p_fd;
 	struct file *fp;
 	struct vnode *vp;
-	int i, tmp, newmin, flg = F_POSIX;
+	int i, prev, tmp, newmin, flg = F_POSIX;
 	struct flock fl;
 	int error = 0;
 
@@ -480,8 +480,11 @@ restart:
 		break;
 
 	case F_SETFL:
-		fp->f_flag &= ~FCNTLFLAGS;
-		fp->f_flag |= FFLAGS((long)SCARG(uap, arg)) & FCNTLFLAGS;
+		do {
+			tmp = prev = fp->f_flag;
+			tmp &= ~FCNTLFLAGS;
+			tmp |= FFLAGS((long)SCARG(uap, arg)) & FCNTLFLAGS;
+		} while (atomic_cas_uint(&fp->f_flag, prev, tmp) != prev);
 		tmp = fp->f_flag & FNONBLOCK;
 		error = (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&tmp, p);
 		if (error)
@@ -490,7 +493,7 @@ restart:
 		error = (*fp->f_ops->fo_ioctl)(fp, FIOASYNC, (caddr_t)&tmp, p);
 		if (!error)
 			break;
-		fp->f_flag &= ~FNONBLOCK;
+		atomic_clearbits_int(&fp->f_flag, FNONBLOCK);
 		tmp = 0;
 		(void) (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&tmp, p);
 		break;
diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index dc050095a33..7a5a1a18749 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sys_generic.c,v 1.128 2020/01/16 16:35:04 mpi Exp $	*/
+/*	$OpenBSD: sys_generic.c,v 1.129 2020/02/01 08:57:27 anton Exp $	*/
 /*	$NetBSD: sys_generic.c,v 1.24 1996/03/29 00:25:32 cgd Exp $	*/
 
 /*
@@ -475,17 +475,17 @@ sys_ioctl(struct proc *p, void *v, register_t *retval)
 
 	case FIONBIO:
 		if ((tmp = *(int *)data) != 0)
-			fp->f_flag |= FNONBLOCK;
+			atomic_setbits_int(&fp->f_flag, FNONBLOCK);
 		else
-			fp->f_flag &= ~FNONBLOCK;
+			atomic_clearbits_int(&fp->f_flag, FNONBLOCK);
 		error = (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&tmp, p);
 		break;
 
 	case FIOASYNC:
 		if ((tmp = *(int *)data) != 0)
-			fp->f_flag |= FASYNC;
+			atomic_setbits_int(&fp->f_flag, FASYNC);
 		else
-			fp->f_flag &= ~FASYNC;
+			atomic_clearbits_int(&fp->f_flag, FASYNC);
 		error = (*fp->f_ops->fo_ioctl)(fp, FIOASYNC, (caddr_t)&tmp, p);
 		break;
 
diff --git a/sys/sys/file.h b/sys/sys/file.h
index 540ad6f7bf7..c52d9c8e089 100644
--- a/sys/sys/file.h
+++ b/sys/sys/file.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: file.h,v 1.59 2020/01/05 13:46:02 visa Exp $	*/
+/*	$OpenBSD: file.h,v 1.60 2020/02/01 08:57:27 anton Exp $	*/
 /*	$NetBSD: file.h,v 1.11 1995/03/26 20:24:13 jtc Exp $	*/
 
 /*
@@ -80,19 +80,19 @@ struct	fileops {
 struct file {
 	LIST_ENTRY(file) f_list;/* [F] list of active files */
 	struct mutex f_mtx;
-	short	f_flag;		/* [k] see fcntl.h */
+	u_int	f_flag;		/* [a] see fcntl.h */
 #define	DTYPE_VNODE	1	/* file */
 #define	DTYPE_SOCKET	2	/* communications endpoint */
 #define	DTYPE_PIPE	3	/* pipe */
 #define	DTYPE_KQUEUE	4	/* event queue */
 #define	DTYPE_DMABUF	5	/* DMA buffer (for DRM) */
-	short	f_type;		/* [I] descriptor type */
+	int	f_iflags;	/* [k] internal flags */
+	int	f_type;		/* [I] descriptor type */
 	u_int	f_count;	/* [a] reference count */
 	struct	ucred *f_cred;	/* [I] credentials associated with descriptor */
 	const struct fileops *f_ops; /* [I] file operation pointers */
 	off_t	f_offset;	/* [f,v] offset */
 	void 	*f_data;	/* [I] private data */
-	int	f_iflags;	/* [k] internal flags */
 	uint64_t f_rxfer;	/* [f] total number of read transfers */
 	uint64_t f_wxfer;	/* [f] total number of write transfers */
 	uint64_t f_seek;	/* [f] total independent seek operations */
diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
index c3b877aad04..efd06a9738a 100644
--- a/sys/sys/sysctl.h
+++ b/sys/sys/sysctl.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sysctl.h,v 1.201 2020/01/30 08:51:27 mpi Exp $	*/
+/*	$OpenBSD: sysctl.h,v 1.202 2020/02/01 08:57:27 anton Exp $	*/
 /*	$NetBSD: sysctl.h,v 1.16 1996/04/09 20:55:36 cgd Exp $	*/
 
 /*
@@ -734,7 +734,7 @@ do {									\
 
 struct kinfo_file {
 	uint64_t	f_fileaddr;	/* PTR: address of struct file */
-	uint32_t	f_flag;		/* SHORT: flags (see fcntl.h) */
+	uint32_t	f_flag;		/* UINT: flags (see fcntl.h) */
 	uint32_t	f_iflags;	/* INT: internal flags */
 	uint32_t	f_type;		/* INT: descriptor type */
 	uint32_t	f_count;	/* UINT: reference count */