Date: Tue, 21 Oct 2014 19:23:06 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: arch@freebsd.org Subject: Re: RfC: fueword(9) and casueword(9) Message-ID: <20141021162306.GE1877@kib.kiev.ua> In-Reply-To: <20141022002825.H2080@besplex.bde.org> References: <20141021094539.GA1877@kib.kiev.ua> <20141022002825.H2080@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 22, 2014 at 01:41:12AM +1100, Bruce Evans wrote:
> On Tue, 21 Oct 2014, Konstantin Belousov wrote:
> This API is larger, slower, and harder to use. No better fix is evident,
> except for fubyte() and fuword16(). These never had a problem on any
> supported arch, since bytes are only 8 bits on all supported arches,
> and 16-bit ints are not supported on any arch, so -1 is always out of
> band. Not touching them is a better fix. You didn't change any of
> their callers, but pessimized their implementation to a wrapper around
> fue*(). (BTW, fuword16() and fuswintr() are misdocumented as taking a
> non-const void * arg.).
I reverted the addition of fuebyte(9) and fueword16(9). First I thought
that it would be nicer to provide fully complement KPI with regard to
fuX/fueX, but it seems that lack of fuebyte() and fueword16() will
cause the right questions.
> > @@ -921,7 +933,9 @@ do_lock_normal(struct thread *td, struct umutex *m, uint32_t flags,
> > * can fault on any access.
> > */
> > for (;;) {
> > - owner = fuword32(__DEVOLATILE(void *, &m->m_owner));
> > + rv = fueword32(__DEVOLATILE(void *, &m->m_owner), &owner);
> > + if (error == -1)
> > + return (EFAULT);
> > if (mode == _UMUTEX_WAIT) {
> > if (owner == UMUTEX_UNOWNED || owner == UMUTEX_CONTESTED)
> > return (0);
>
> A new API should try to fix these __DEVOLATILE() abominations. I think it
> is safe, and even correct, to declare the pointers as volatile const void *,
> since the functions really can handle volatile data, unlike copyin().
>
> Atomic op functions are declared as taking pointers to volatile for
> similar reasons. Often they are applied to non-volatile data, but
> adding a qualifier is type-safe and doesn't cost efficiency since the
> pointer access is is not known to the compiler. (The last point is not
> so clear -- the compiler can see things in the functions since they are
> inline asm. fueword() isn't inline so its (in)efficiency is not changed.)
>
> The atomic read functions are not declared as taking pointers to const.
> The __DECONST() abomination might be used to work around this bug.
I prefer to not complicate the fetch(9) KPI due to the mistakes in the
umtx structures definitions. I think that it is bug to mark the lock
words with volatile. I want the fueword(9) interface to be as much
similar to fuword(9), in particular, volatile seems to be not needed.
Below is the updated patch, together with the bug fix noted by Eric.
diff --git a/share/man/man9/Makefile b/share/man/man9/Makefile
index bc21dc6..0464aca 100644
--- a/share/man/man9/Makefile
+++ b/share/man/man9/Makefile
@@ -581,6 +581,9 @@ MLINKS+=condvar.9 cv_broadcast.9 \
MLINKS+=config_intrhook.9 config_intrhook_disestablish.9 \
config_intrhook.9 config_intrhook_establish.9
MLINKS+=contigmalloc.9 contigfree.9
+MLINKS+=casuword.9 casueword.9 \
+ casuword.9 casueword32.9 \
+ casuword.9 casuword32.9
MLINKS+=copy.9 copyin.9 \
copy.9 copyin_nofault.9 \
copy.9 copyinstr.9 \
@@ -688,7 +691,12 @@ MLINKS+=fetch.9 fubyte.9 \
fetch.9 fuword.9 \
fetch.9 fuword16.9 \
fetch.9 fuword32.9 \
- fetch.9 fuword64.9
+ fetch.9 fuword64.9 \
+ fetch.9 fuebyte.9 \
+ fetch.9 fueword.9 \
+ fetch.9 fueword16.9 \
+ fetch.9 fueword32.9 \
+ fetch.9 fueword64.9
MLINKS+=firmware.9 firmware_get.9 \
firmware.9 firmware_put.9 \
firmware.9 firmware_register.9 \
diff --git a/share/man/man9/casuword.9 b/share/man/man9/casuword.9
new file mode 100644
index 0000000..34a0f1d
--- /dev/null
+++ b/share/man/man9/casuword.9
@@ -0,0 +1,95 @@
+.\" Copyright (c) 2014 The FreeBSD Foundation
+.\" All rights reserved.
+.\"
+.\" Part of this documentation was written by
+.\" Konstantin Belousov <kib@FreeBSD.org> under sponsorship
+.\" from the FreeBSD Foundation.
+.\"
+.\" Redistribution and use in source and binary forms, with or without
+.\" modification, are permitted provided that the following conditions
+.\" are met:
+.\" 1. Redistributions of source code must retain the above copyright
+.\" notice, this list of conditions and the following disclaimer.
+.\" 2. Redistributions in binary form must reproduce the above copyright
+.\" notice, this list of conditions and the following disclaimer in the
+.\" documentation and/or other materials provided with the distribution.
+.\"
+.\" THIS SOFTWARE IS PROVIDED BY THE AUTHORS AND CONTRIBUTORS ``AS IS'' AND
+.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+.\" ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE
+.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+.\" SUCH DAMAGE.
+.\"
+.\" $FreeBSD$
+.\"
+.Dd October 21, 2014
+.Dt CASU 9
+.Os
+.Sh NAME
+.Nm casueword ,
+.Nm casueword32 ,
+.Nm casuword ,
+.Nm casuword32
+.Nd fetch, compare and store data from user-space
+.Sh SYNOPSIS
+.In sys/types.h
+.In sys/systm.h
+.Ft int
+.Fn casueword "volatile u_long *base" "u_long oldval" "u_long *oldvalp" "u_long newval"
+.Ft int
+.Fn casueword32 "volatile uint32_t *base" "uint32_t oldval" "uint32_t *oldvalp" "uint32_t newval"
+.Ft u_long
+.Fn casuword "volatile u_long *base" "u_long oldval" "u_long newval"
+.Ft uint32_t
+.Fn casuword32 "volatile uint32_t *base" "uint32_t oldval" "uint32_t newval"
+.Sh DESCRIPTION
+The
+.Nm
+functions are designed to perform atomic compare-and-swap operation on
+the value in the usermode memory of the current process.
+.Pp
+The
+.Nm
+routines reads the value from user memory with address
+.Pa base ,
+and compare the value read with
+.Pa oldval .
+If the values are equal,
+.Pa newval
+is written to the
+.Pa *base .
+In case of
+.Fn casueword32
+and
+.Fn casueword ,
+old value is stored into the (kernel-mode) variable pointed by
+.Pa *oldvalp .
+The userspace value must be naturally aligned.
+.Pp
+The callers of
+.Fn casuword
+and
+.Fn casuword32
+functions cannot distinguish between -1 read from
+userspace and function failure.
+.Sh RETURN VALUES
+The
+.Fn casuword
+and
+.Fn casuword32
+functions return the data fetched or -1 on failure.
+The
+.Fn casueword
+and
+.Fn casueword32
+functions return 0 on success and -1 on failure.
+.Sh SEE ALSO
+.Xr atomic 9 ,
+.Xr fetch 9 ,
+.Xr store 9
diff --git a/share/man/man9/fetch.9 b/share/man/man9/fetch.9
index ccf6866..e8ff100 100644
--- a/share/man/man9/fetch.9
+++ b/share/man/man9/fetch.9
@@ -34,7 +34,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd October 5, 2009
+.Dd October 21, 2014
.Dt FETCH 9
.Os
.Sh NAME
@@ -44,11 +44,13 @@
.Nm fuword ,
.Nm fuword16 ,
.Nm fuword32 ,
-.Nm fuword64
+.Nm fuword64 ,
+.Nm fueword ,
+.Nm fueword32 ,
+.Nm fueword64
.Nd fetch data from user-space
.Sh SYNOPSIS
.In sys/types.h
-.In sys/time.h
.In sys/systm.h
.Ft int
.Fn fubyte "const void *base"
@@ -60,18 +62,27 @@
.Fn fuword32 "const void *base"
.Ft int64_t
.Fn fuword64 "const void *base"
+.Ft long
+.Fn fueword "const void *base" "long *val"
+.Ft int32_t
+.Fn fueword32 "const void *base" "int32_t *val"
+.Ft int64_t
+.Fn fueword64 "const void *base" "int64_t *val"
.In sys/resourcevar.h
.Ft int
.Fn fuswintr "void *base"
.Sh DESCRIPTION
The
.Nm
-functions are designed to copy small amounts of data from user-space.
+functions are designed to copy small amounts of data from user-space
+of the current process.
+If read is successfull, it is performed atomically.
+The data read must be naturally aligned.
.Pp
The
.Nm
routines provide the following functionality:
-.Bl -tag -width "fuswintr()"
+.Bl -tag -width "fueword32()"
.It Fn fubyte
Fetches a byte of data from the user-space address
.Pa base .
@@ -91,11 +102,46 @@ Fetches 64 bits of data from the user-space address
Fetches a short word of data from the user-space address
.Pa base .
This function is safe to call during an interrupt context.
+.It Fn fueword
+Fetches a word of data from the user-space address
+.Pa base
+and stores the result in the variable pointed by
+.Pa val .
+.It Fn fueword32
+Fetches 32 bits of data from the user-space address
+.Pa base
+and stores the result in the variable pointed by
+.Pa val .
+.It Fn fueword64
+Fetches 64 bits of data from the user-space address
+.Pa base
+and stores the result in the variable pointed by
+.Pa val .
.El
+.Pp
+The callers of
+.Fn fuword ,
+.Fn fuword32
+and
+.Fn fuword64
+functions cannot distinguish between -1 read from
+userspace and function failure.
.Sh RETURN VALUES
The
-.Nm
+.Fn fubyte ,
+.Fn fuword ,
+.Fn fuword16 ,
+.Fn fuword32 ,
+.Fn fuword64 ,
+and
+.Fn fuswintr
functions return the data fetched or -1 on failure.
+The
+.Fn fueword ,
+.Fn fueword32
+and
+.Fn fueword64
+functions return 0 on success and -1 on failure.
.Sh SEE ALSO
.Xr copy 9 ,
.Xr store 9
diff --git a/sys/amd64/amd64/support.S b/sys/amd64/amd64/support.S
index 4897367..f9a3165 100644
--- a/sys/amd64/amd64/support.S
+++ b/sys/amd64/amd64/support.S
@@ -312,12 +312,13 @@ copyin_fault:
END(copyin)
/*
- * casuword32. Compare and set user integer. Returns -1 or the current value.
- * dst = %rdi, old = %rsi, new = %rdx
+ * casueword32. Compare and set user integer. Returns -1 on fault,
+ * 0 if access was successfull. Old value is written to *oldp.
+ * dst = %rdi, old = %esi, oldp = %rdx, new = %ecx
*/
-ENTRY(casuword32)
- movq PCPU(CURPCB),%rcx
- movq $fusufault,PCB_ONFAULT(%rcx)
+ENTRY(casueword32)
+ movq PCPU(CURPCB),%r8
+ movq $fusufault,PCB_ONFAULT(%r8)
movq $VM_MAXUSER_ADDRESS-4,%rax
cmpq %rax,%rdi /* verify address is valid */
@@ -327,26 +328,34 @@ ENTRY(casuword32)
#ifdef SMP
lock
#endif
- cmpxchgl %edx,(%rdi) /* new = %edx */
+ cmpxchgl %ecx,(%rdi) /* new = %ecx */
/*
* The old value is in %eax. If the store succeeded it will be the
* value we expected (old) from before the store, otherwise it will
- * be the current value.
+ * be the current value. Save %eax into %esi to prepare the return
+ * value.
*/
+ movl %eax,%esi
+ xorl %eax,%eax
+ movq %rax,PCB_ONFAULT(%r8)
- movq PCPU(CURPCB),%rcx
- movq $0,PCB_ONFAULT(%rcx)
+ /*
+ * Access the oldp after the pcb_onfault is cleared, to correctly
+ * catch corrupted pointer.
+ */
+ movl %esi,(%rdx) /* oldp = %rdx */
ret
-END(casuword32)
+END(casueword32)
/*
- * casuword. Compare and set user word. Returns -1 or the current value.
- * dst = %rdi, old = %rsi, new = %rdx
+ * casueword. Compare and set user long. Returns -1 on fault,
+ * 0 if access was successfull. Old value is written to *oldp.
+ * dst = %rdi, old = %rsi, oldp = %rdx, new = %rcx
*/
-ENTRY(casuword)
- movq PCPU(CURPCB),%rcx
- movq $fusufault,PCB_ONFAULT(%rcx)
+ENTRY(casueword)
+ movq PCPU(CURPCB),%r8
+ movq $fusufault,PCB_ONFAULT(%r8)
movq $VM_MAXUSER_ADDRESS-4,%rax
cmpq %rax,%rdi /* verify address is valid */
@@ -356,28 +365,28 @@ ENTRY(casuword)
#ifdef SMP
lock
#endif
- cmpxchgq %rdx,(%rdi) /* new = %rdx */
+ cmpxchgq %rcx,(%rdi) /* new = %rcx */
/*
- * The old value is in %eax. If the store succeeded it will be the
+ * The old value is in %rax. If the store succeeded it will be the
* value we expected (old) from before the store, otherwise it will
* be the current value.
*/
-
- movq PCPU(CURPCB),%rcx
- movq $fusufault,PCB_ONFAULT(%rcx)
- movq $0,PCB_ONFAULT(%rcx)
+ movq %rax,%rsi
+ xorl %eax,%eax
+ movq %rax,PCB_ONFAULT(%r8)
+ movq %rsi,(%rdx)
ret
-END(casuword)
+END(casueword)
/*
* Fetch (load) a 64-bit word, a 32-bit word, a 16-bit word, or an 8-bit
- * byte from user memory. All these functions are MPSAFE.
- * addr = %rdi
+ * byte from user memory.
+ * addr = %rdi, valp = %rsi
*/
-ALTENTRY(fuword64)
-ENTRY(fuword)
+ALTENTRY(fueword64)
+ENTRY(fueword)
movq PCPU(CURPCB),%rcx
movq $fusufault,PCB_ONFAULT(%rcx)
@@ -385,13 +394,15 @@ ENTRY(fuword)
cmpq %rax,%rdi /* verify address is valid */
ja fusufault
- movq (%rdi),%rax
- movq $0,PCB_ONFAULT(%rcx)
+ xorl %eax,%eax
+ movq (%rdi),%r11
+ movq %rax,PCB_ONFAULT(%rcx)
+ movq %r11,(%rsi)
ret
END(fuword64)
END(fuword)
-ENTRY(fuword32)
+ENTRY(fueword32)
movq PCPU(CURPCB),%rcx
movq $fusufault,PCB_ONFAULT(%rcx)
@@ -399,10 +410,12 @@ ENTRY(fuword32)
cmpq %rax,%rdi /* verify address is valid */
ja fusufault
- movl (%rdi),%eax
- movq $0,PCB_ONFAULT(%rcx)
+ xorl %eax,%eax
+ movl (%rdi),%r11d
+ movq %rax,PCB_ONFAULT(%rcx)
+ movl %r11d,(%rsi)
ret
-END(fuword32)
+END(fueword32)
/*
* fuswintr() and suswintr() are specialized variants of fuword16() and
diff --git a/sys/amd64/ia32/ia32_syscall.c b/sys/amd64/ia32/ia32_syscall.c
index 0cdec6f..92249f9 100644
--- a/sys/amd64/ia32/ia32_syscall.c
+++ b/sys/amd64/ia32/ia32_syscall.c
@@ -110,7 +110,7 @@ ia32_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
struct proc *p;
struct trapframe *frame;
caddr_t params;
- u_int32_t args[8];
+ u_int32_t args[8], tmp;
int error, i;
p = td->td_proc;
@@ -126,7 +126,10 @@ ia32_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
/*
* Code is first argument, followed by actual args.
*/
- sa->code = fuword32(params);
+ error = fueword32(params, &tmp);
+ if (error == -1)
+ return (EFAULT);
+ sa->code = tmp;
params += sizeof(int);
} else if (sa->code == SYS___syscall) {
/*
@@ -135,7 +138,10 @@ ia32_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
* We use a 32-bit fetch in case params is not
* aligned.
*/
- sa->code = fuword32(params);
+ error = fueword32(params, &tmp);
+ if (error == -1)
+ return (EFAULT);
+ sa->code = tmp;
params += sizeof(quad_t);
}
if (p->p_sysent->sv_mask)
diff --git a/sys/arm/include/param.h b/sys/arm/include/param.h
index 4a64607..6267154 100644
--- a/sys/arm/include/param.h
+++ b/sys/arm/include/param.h
@@ -149,4 +149,8 @@
#define pgtok(x) ((x) * (PAGE_SIZE / 1024))
+#ifdef _KERNEL
+#define NO_FUEWORD 1
+#endif
+
#endif /* !_ARM_INCLUDE_PARAM_H_ */
diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c
index d909a71..98948d3 100644
--- a/sys/compat/freebsd32/freebsd32_misc.c
+++ b/sys/compat/freebsd32/freebsd32_misc.c
@@ -1832,16 +1832,21 @@ freebsd32_sysctl(struct thread *td, struct freebsd32_sysctl_args *uap)
{
int error, name[CTL_MAXNAME];
size_t j, oldlen;
+ uint32_t tmp;
if (uap->namelen > CTL_MAXNAME || uap->namelen < 2)
return (EINVAL);
error = copyin(uap->name, name, uap->namelen * sizeof(int));
if (error)
return (error);
- if (uap->oldlenp)
- oldlen = fuword32(uap->oldlenp);
- else
+ if (uap->oldlenp) {
+ error = fueword32(uap->oldlenp, &tmp);
+ oldlen = tmp;
+ } else {
oldlen = 0;
+ }
+ if (error != 0)
+ return (EFAULT);
error = userland_sysctl(td, name, uap->namelen,
uap->old, &oldlen, 1,
uap->new, uap->newlen, &j, SCTL_MASK32);
diff --git a/sys/i386/i386/support.s b/sys/i386/i386/support.s
index c126f78..0a08012 100644
--- a/sys/i386/i386/support.s
+++ b/sys/i386/i386/support.s
@@ -389,16 +389,16 @@ copyin_fault:
ret
/*
- * casuword. Compare and set user word. Returns -1 or the current value.
+ * casueword. Compare and set user word. Returns -1 on fault,
+ * 0 on non-faulting access. The current value is in *oldp.
*/
-
-ALTENTRY(casuword32)
-ENTRY(casuword)
+ALTENTRY(casueword32)
+ENTRY(casueword)
movl PCPU(CURPCB),%ecx
movl $fusufault,PCB_ONFAULT(%ecx)
movl 4(%esp),%edx /* dst */
movl 8(%esp),%eax /* old */
- movl 12(%esp),%ecx /* new */
+ movl 16(%esp),%ecx /* new */
cmpl $VM_MAXUSER_ADDRESS-4,%edx /* verify address is valid */
ja fusufault
@@ -416,17 +416,20 @@ ENTRY(casuword)
movl PCPU(CURPCB),%ecx
movl $0,PCB_ONFAULT(%ecx)
+ movl 12(%esp),%edx /* oldp */
+ movl %eax,(%edx)
+ xorl %eax,%eax
ret
-END(casuword32)
-END(casuword)
+END(casueword32)
+END(casueword)
/*
* Fetch (load) a 32-bit word, a 16-bit word, or an 8-bit byte from user
- * memory. All these functions are MPSAFE.
+ * memory.
*/
-ALTENTRY(fuword32)
-ENTRY(fuword)
+ALTENTRY(fueword32)
+ENTRY(fueword)
movl PCPU(CURPCB),%ecx
movl $fusufault,PCB_ONFAULT(%ecx)
movl 4(%esp),%edx /* from */
@@ -436,9 +439,12 @@ ENTRY(fuword)
movl (%edx),%eax
movl $0,PCB_ONFAULT(%ecx)
+ movl 8(%esp),%edx
+ movl %eax,(%edx)
+ xorl %eax,%eax
ret
-END(fuword32)
-END(fuword)
+END(fueword32)
+END(fueword)
/*
* fuswintr() and suswintr() are specialized variants of fuword16() and
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index 1d0d104..84d6ec3 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -1059,6 +1059,7 @@ cpu_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
struct proc *p;
struct trapframe *frame;
caddr_t params;
+ long tmp;
int error;
p = td->td_proc;
@@ -1074,14 +1075,20 @@ cpu_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
/*
* Code is first argument, followed by actual args.
*/
- sa->code = fuword(params);
+ error = fueword(params, &tmp);
+ if (error == -1)
+ return (EFAULT);
+ sa->code = tmp;
params += sizeof(int);
} else if (sa->code == SYS___syscall) {
/*
* Like syscall, but code is a quad, so as to maintain
* quad alignment for the rest of the arguments.
*/
- sa->code = fuword(params);
+ error = fueword(params, &tmp);
+ if (error == -1)
+ return (EFAULT);
+ sa->code = tmp;
params += sizeof(quad_t);
}
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index f2bbdaa..13a52e9 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1091,7 +1091,7 @@ int
exec_copyin_args(struct image_args *args, char *fname,
enum uio_seg segflg, char **argv, char **envv)
{
- char *argp, *envp;
+ u_long argp, envp;
int error;
size_t length;
@@ -1127,13 +1127,17 @@ exec_copyin_args(struct image_args *args, char *fname,
/*
* extract arguments first
*/
- while ((argp = (caddr_t) (intptr_t) fuword(argv++))) {
- if (argp == (caddr_t) -1) {
+ for (;;) {
+ error = fueword(argv++, &argp);
+ if (error == -1) {
error = EFAULT;
goto err_exit;
}
- if ((error = copyinstr(argp, args->endp,
- args->stringspace, &length))) {
+ if (argp == 0)
+ break;
+ error = copyinstr((void *)(uintptr_t)argp, args->endp,
+ args->stringspace, &length);
+ if (error != 0) {
if (error == ENAMETOOLONG)
error = E2BIG;
goto err_exit;
@@ -1149,13 +1153,17 @@ exec_copyin_args(struct image_args *args, char *fname,
* extract environment strings
*/
if (envv) {
- while ((envp = (caddr_t)(intptr_t)fuword(envv++))) {
- if (envp == (caddr_t)-1) {
+ for (;;) {
+ error = fueword(envv++, &envp);
+ if (error == -1) {
error = EFAULT;
goto err_exit;
}
- if ((error = copyinstr(envp, args->endp,
- args->stringspace, &length))) {
+ if (envp == 0)
+ break;
+ error = copyinstr((void *)(uintptr_t)envp,
+ args->endp, args->stringspace, &length);
+ if (error != 0) {
if (error == ENAMETOOLONG)
error = E2BIG;
goto err_exit;
diff --git a/sys/kern/kern_umtx.c b/sys/kern/kern_umtx.c
index 7cfef38..f5c4cc2 100644
--- a/sys/kern/kern_umtx.c
+++ b/sys/kern/kern_umtx.c
@@ -847,6 +847,7 @@ do_wait(struct thread *td, void *addr, u_long id,
struct abs_timeout timo;
struct umtx_q *uq;
u_long tmp;
+ uint32_t tmp32;
int error = 0;
uq = td->td_umtxq;
@@ -860,18 +861,29 @@ do_wait(struct thread *td, void *addr, u_long id,
umtxq_lock(&uq->uq_key);
umtxq_insert(uq);
umtxq_unlock(&uq->uq_key);
- if (compat32 == 0)
- tmp = fuword(addr);
- else
- tmp = (unsigned int)fuword32(addr);
+ if (compat32 == 0) {
+ error = fueword(addr, &tmp);
+ if (error != 0)
+ error = EFAULT;
+ } else {
+ error = fueword32(addr, &tmp32);
+ if (error == 0)
+ tmp = tmp32;
+ else
+ error = EFAULT;
+ }
umtxq_lock(&uq->uq_key);
- if (tmp == id)
- error = umtxq_sleep(uq, "uwait", timeout == NULL ?
- NULL : &timo);
- if ((uq->uq_flags & UQF_UMTXQ) == 0)
- error = 0;
- else
+ if (error == 0) {
+ if (tmp == id)
+ error = umtxq_sleep(uq, "uwait", timeout == NULL ?
+ NULL : &timo);
+ if ((uq->uq_flags & UQF_UMTXQ) == 0)
+ error = 0;
+ else
+ umtxq_remove(uq);
+ } else if ((uq->uq_flags & UQF_UMTXQ) != 0) {
umtxq_remove(uq);
+ }
umtxq_unlock(&uq->uq_key);
umtx_key_release(&uq->uq_key);
if (error == ERESTART)
@@ -908,11 +920,11 @@ do_lock_normal(struct thread *td, struct umutex *m, uint32_t flags,
struct abs_timeout timo;
struct umtx_q *uq;
uint32_t owner, old, id;
- int error = 0;
+ int error, rv;
id = td->td_tid;
uq = td->td_umtxq;
-
+ error = 0;
if (timeout != NULL)
abs_timeout_init2(&timo, timeout);
@@ -921,7 +933,9 @@ do_lock_normal(struct thread *td, struct umutex *m, uint32_t flags,
* can fault on any access.
*/
for (;;) {
- owner = fuword32(__DEVOLATILE(void *, &m->m_owner));
+ rv = fueword32(__DEVOLATILE(void *, &m->m_owner), &owner);
+ if (rv == -1)
+ return (EFAULT);
if (mode == _UMUTEX_WAIT) {
if (owner == UMUTEX_UNOWNED || owner == UMUTEX_CONTESTED)
return (0);
@@ -929,31 +943,31 @@ do_lock_normal(struct thread *td, struct umutex *m, uint32_t flags,
/*
* Try the uncontested case. This should be done in userland.
*/
- owner = casuword32(&m->m_owner, UMUTEX_UNOWNED, id);
+ rv = casueword32(&m->m_owner, UMUTEX_UNOWNED,
+ &owner, id);
+ /* The address was invalid. */
+ if (rv == -1)
+ return (EFAULT);
/* The acquire succeeded. */
if (owner == UMUTEX_UNOWNED)
return (0);
- /* The address was invalid. */
- if (owner == -1)
- return (EFAULT);
-
/* If no one owns it but it is contested try to acquire it. */
if (owner == UMUTEX_CONTESTED) {
- owner = casuword32(&m->m_owner,
- UMUTEX_CONTESTED, id | UMUTEX_CONTESTED);
+ rv = casueword32(&m->m_owner,
+ UMUTEX_CONTESTED, &owner,
+ id | UMUTEX_CONTESTED);
+ /* The address was invalid. */
+ if (rv == -1)
+ return (EFAULT);
if (owner == UMUTEX_CONTESTED)
return (0);
- /* The address was invalid. */
- if (owner == -1)
- return (EFAULT);
-
- error = umtxq_check_susp(td);
- if (error != 0)
- return (error);
+ rv = umtxq_check_susp(td);
+ if (rv != 0)
+ return (rv);
/* If this failed the lock has changed, restart. */
continue;
@@ -985,10 +999,11 @@ do_lock_normal(struct thread *td, struct umutex *m, uint32_t flags,
* either some one else has acquired the lock or it has been
* released.
*/
- old = casuword32(&m->m_owner, owner, owner | UMUTEX_CONTESTED);
+ rv = casueword32(&m->m_owner, owner, &old,
+ owner | UMUTEX_CONTESTED);
/* The address was invalid. */
- if (old == -1) {
+ if (rv == -1) {
umtxq_lock(&uq->uq_key);
umtxq_remove(uq);
umtxq_unbusy(&uq->uq_key);
@@ -1033,16 +1048,16 @@ do_unlock_normal(struct thread *td, struct umutex *m, uint32_t flags)
/*
* Make sure we own this mtx.
*/
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- if (owner == -1)
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner), &owner);
+ if (error == -1)
return (EFAULT);
if ((owner & ~UMUTEX_CONTESTED) != id)
return (EPERM);
if ((owner & UMUTEX_CONTESTED) == 0) {
- old = casuword32(&m->m_owner, owner, UMUTEX_UNOWNED);
- if (old == -1)
+ error = casueword32(&m->m_owner, owner, &old, UMUTEX_UNOWNED);
+ if (error == -1)
return (EFAULT);
if (old == owner)
return (0);
@@ -1064,14 +1079,14 @@ do_unlock_normal(struct thread *td, struct umutex *m, uint32_t flags)
* there is zero or one thread only waiting for it.
* Otherwise, it must be marked as contested.
*/
- old = casuword32(&m->m_owner, owner,
+ error = casueword32(&m->m_owner, owner, &old,
count <= 1 ? UMUTEX_UNOWNED : UMUTEX_CONTESTED);
umtxq_lock(&key);
umtxq_signal(&key,1);
umtxq_unbusy(&key);
umtxq_unlock(&key);
umtx_key_release(&key);
- if (old == -1)
+ if (error == -1)
return (EFAULT);
if (old != owner)
return (EINVAL);
@@ -1091,14 +1106,16 @@ do_wake_umutex(struct thread *td, struct umutex *m)
int error;
int count;
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- if (owner == -1)
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner), &owner);
+ if (error == -1)
return (EFAULT);
if ((owner & ~UMUTEX_CONTESTED) != 0)
return (0);
- flags = fuword32(&m->m_flags);
+ error = fueword32(&m->m_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
/* We should only ever be in here for contested locks */
if ((error = umtx_key_get(m, TYPE_NORMAL_UMUTEX, GET_SHARE(flags),
@@ -1110,16 +1127,20 @@ do_wake_umutex(struct thread *td, struct umutex *m)
count = umtxq_count(&key);
umtxq_unlock(&key);
- if (count <= 1)
- owner = casuword32(&m->m_owner, UMUTEX_CONTESTED, UMUTEX_UNOWNED);
+ if (count <= 1) {
+ error = casueword32(&m->m_owner, UMUTEX_CONTESTED, &owner,
+ UMUTEX_UNOWNED);
+ if (error == -1)
+ error = EFAULT;
+ }
umtxq_lock(&key);
- if (count != 0 && (owner & ~UMUTEX_CONTESTED) == 0)
+ if (error == 0 && count != 0 && (owner & ~UMUTEX_CONTESTED) == 0)
umtxq_signal(&key, 1);
umtxq_unbusy(&key);
umtxq_unlock(&key);
umtx_key_release(&key);
- return (0);
+ return (error);
}
/*
@@ -1162,41 +1183,49 @@ do_wake2_umutex(struct thread *td, struct umutex *m, uint32_t flags)
* any memory.
*/
if (count > 1) {
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- while ((owner & UMUTEX_CONTESTED) ==0) {
- old = casuword32(&m->m_owner, owner,
- owner|UMUTEX_CONTESTED);
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner),
+ &owner);
+ if (error == -1)
+ error = EFAULT;
+ while (error == 0 && (owner & UMUTEX_CONTESTED) == 0) {
+ error = casueword32(&m->m_owner, owner, &old,
+ owner | UMUTEX_CONTESTED);
+ if (error == -1) {
+ error = EFAULT;
+ break;
+ }
if (old == owner)
break;
owner = old;
- if (old == -1)
- break;
error = umtxq_check_susp(td);
if (error != 0)
break;
}
} else if (count == 1) {
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- while ((owner & ~UMUTEX_CONTESTED) != 0 &&
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner),
+ &owner);
+ if (error == -1)
+ error = EFAULT;
+ while (error == 0 && (owner & ~UMUTEX_CONTESTED) != 0 &&
(owner & UMUTEX_CONTESTED) == 0) {
- old = casuword32(&m->m_owner, owner,
- owner|UMUTEX_CONTESTED);
+ error = casueword32(&m->m_owner, owner, &old,
+ owner | UMUTEX_CONTESTED);
+ if (error == -1) {
+ error = EFAULT;
+ break;
+ }
if (old == owner)
break;
owner = old;
- if (old == -1)
- break;
error = umtxq_check_susp(td);
if (error != 0)
break;
}
}
umtxq_lock(&key);
- if (owner == -1) {
- error = EFAULT;
+ if (error == EFAULT) {
umtxq_signal(&key, INT_MAX);
- }
- else if (count != 0 && (owner & ~UMUTEX_CONTESTED) == 0)
+ } else if (count != 0 && (owner & ~UMUTEX_CONTESTED) == 0)
umtxq_signal(&key, 1);
umtxq_unbusy(&key);
umtxq_unlock(&key);
@@ -1576,7 +1605,7 @@ do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags,
struct umtx_q *uq;
struct umtx_pi *pi, *new_pi;
uint32_t id, owner, old;
- int error;
+ int error, rv;
id = td->td_tid;
uq = td->td_umtxq;
@@ -1619,7 +1648,12 @@ do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags,
/*
* Try the uncontested case. This should be done in userland.
*/
- owner = casuword32(&m->m_owner, UMUTEX_UNOWNED, id);
+ rv = casueword32(&m->m_owner, UMUTEX_UNOWNED, &owner, id);
+ /* The address was invalid. */
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
/* The acquire succeeded. */
if (owner == UMUTEX_UNOWNED) {
@@ -1627,16 +1661,15 @@ do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags,
break;
}
- /* The address was invalid. */
- if (owner == -1) {
- error = EFAULT;
- break;
- }
-
/* If no one owns it but it is contested try to acquire it. */
if (owner == UMUTEX_CONTESTED) {
- owner = casuword32(&m->m_owner,
- UMUTEX_CONTESTED, id | UMUTEX_CONTESTED);
+ rv = casueword32(&m->m_owner,
+ UMUTEX_CONTESTED, &owner, id | UMUTEX_CONTESTED);
+ /* The address was invalid. */
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
if (owner == UMUTEX_CONTESTED) {
umtxq_lock(&uq->uq_key);
@@ -1647,12 +1680,6 @@ do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags,
break;
}
- /* The address was invalid. */
- if (owner == -1) {
- error = EFAULT;
- break;
- }
-
error = umtxq_check_susp(td);
if (error != 0)
break;
@@ -1683,10 +1710,11 @@ do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags,
* either some one else has acquired the lock or it has been
* released.
*/
- old = casuword32(&m->m_owner, owner, owner | UMUTEX_CONTESTED);
+ rv = casueword32(&m->m_owner, owner, &old,
+ owner | UMUTEX_CONTESTED);
/* The address was invalid. */
- if (old == -1) {
+ if (rv == -1) {
umtxq_lock(&uq->uq_key);
umtxq_unbusy(&uq->uq_key);
umtxq_unlock(&uq->uq_key);
@@ -1741,8 +1769,8 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
/*
* Make sure we own this mtx.
*/
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- if (owner == -1)
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner), &owner);
+ if (error == -1)
return (EFAULT);
if ((owner & ~UMUTEX_CONTESTED) != id)
@@ -1750,8 +1778,8 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
/* This should be done in userland */
if ((owner & UMUTEX_CONTESTED) == 0) {
- old = casuword32(&m->m_owner, owner, UMUTEX_UNOWNED);
- if (old == -1)
+ error = casueword32(&m->m_owner, owner, &old, UMUTEX_UNOWNED);
+ if (error == -1)
return (EFAULT);
if (old == owner)
return (0);
@@ -1809,14 +1837,14 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
* there is zero or one thread only waiting for it.
* Otherwise, it must be marked as contested.
*/
- old = casuword32(&m->m_owner, owner,
- count <= 1 ? UMUTEX_UNOWNED : UMUTEX_CONTESTED);
+ error = casueword32(&m->m_owner, owner, &old,
+ count <= 1 ? UMUTEX_UNOWNED : UMUTEX_CONTESTED);
umtxq_lock(&key);
umtxq_unbusy(&key);
umtxq_unlock(&key);
umtx_key_release(&key);
- if (old == -1)
+ if (error == -1)
return (EFAULT);
if (old != owner)
return (EINVAL);
@@ -1835,7 +1863,7 @@ do_lock_pp(struct thread *td, struct umutex *m, uint32_t flags,
struct umtx_pi *pi;
uint32_t ceiling;
uint32_t owner, id;
- int error, pri, old_inherited_pri, su;
+ int error, pri, old_inherited_pri, su, rv;
id = td->td_tid;
uq = td->td_umtxq;
@@ -1853,7 +1881,12 @@ do_lock_pp(struct thread *td, struct umutex *m, uint32_t flags,
umtxq_busy(&uq->uq_key);
umtxq_unlock(&uq->uq_key);
- ceiling = RTP_PRIO_MAX - fuword32(&m->m_ceilings[0]);
+ rv = fueword32(&m->m_ceilings[0], &ceiling);
+ if (rv == -1) {
+ error = EFAULT;
+ goto out;
+ }
+ ceiling = RTP_PRIO_MAX - ceiling;
if (ceiling > RTP_PRIO_MAX) {
error = EINVAL;
goto out;
@@ -1874,17 +1907,16 @@ do_lock_pp(struct thread *td, struct umutex *m, uint32_t flags,
}
mtx_unlock_spin(&umtx_lock);
- owner = casuword32(&m->m_owner,
- UMUTEX_CONTESTED, id | UMUTEX_CONTESTED);
-
- if (owner == UMUTEX_CONTESTED) {
- error = 0;
+ rv = casueword32(&m->m_owner,
+ UMUTEX_CONTESTED, &owner, id | UMUTEX_CONTESTED);
+ /* The address was invalid. */
+ if (rv == -1) {
+ error = EFAULT;
break;
}
- /* The address was invalid. */
- if (owner == -1) {
- error = EFAULT;
+ if (owner == UMUTEX_CONTESTED) {
+ error = 0;
break;
}
@@ -1973,8 +2005,8 @@ do_unlock_pp(struct thread *td, struct umutex *m, uint32_t flags)
/*
* Make sure we own this mtx.
*/
- owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
- if (owner == -1)
+ error = fueword32(__DEVOLATILE(uint32_t *, &m->m_owner), &owner);
+ if (error == -1)
return (EFAULT);
if ((owner & ~UMUTEX_CONTESTED) != id)
@@ -2047,9 +2079,11 @@ do_set_ceiling(struct thread *td, struct umutex *m, uint32_t ceiling,
uint32_t save_ceiling;
uint32_t owner, id;
uint32_t flags;
- int error;
+ int error, rv;
- flags = fuword32(&m->m_flags);
+ error = fueword32(&m->m_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
if ((flags & UMUTEX_PRIO_PROTECT) == 0)
return (EINVAL);
if (ceiling > RTP_PRIO_MAX)
@@ -2064,10 +2098,18 @@ do_set_ceiling(struct thread *td, struct umutex *m, uint32_t ceiling,
umtxq_busy(&uq->uq_key);
umtxq_unlock(&uq->uq_key);
- save_ceiling = fuword32(&m->m_ceilings[0]);
+ rv = fueword32(&m->m_ceilings[0], &save_ceiling);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
- owner = casuword32(&m->m_owner,
- UMUTEX_CONTESTED, id | UMUTEX_CONTESTED);
+ rv = casueword32(&m->m_owner,
+ UMUTEX_CONTESTED, &owner, id | UMUTEX_CONTESTED);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
if (owner == UMUTEX_CONTESTED) {
suword32(&m->m_ceilings[0], ceiling);
@@ -2077,12 +2119,6 @@ do_set_ceiling(struct thread *td, struct umutex *m, uint32_t ceiling,
break;
}
- /* The address was invalid. */
- if (owner == -1) {
- error = EFAULT;
- break;
- }
-
if ((owner & ~UMUTEX_CONTESTED) == id) {
suword32(&m->m_ceilings[0], ceiling);
error = 0;
@@ -2129,8 +2165,8 @@ do_lock_umutex(struct thread *td, struct umutex *m,
uint32_t flags;
int error;
- flags = fuword32(&m->m_flags);
- if (flags == -1)
+ error = fueword32(&m->m_flags, &flags);
+ if (error == -1)
return (EFAULT);
switch(flags & (UMUTEX_PRIO_INHERIT | UMUTEX_PRIO_PROTECT)) {
@@ -2164,9 +2200,10 @@ static int
do_unlock_umutex(struct thread *td, struct umutex *m)
{
uint32_t flags;
+ int error;
- flags = fuword32(&m->m_flags);
- if (flags == -1)
+ error = fueword32(&m->m_flags, &flags);
+ if (error == -1)
return (EFAULT);
switch(flags & (UMUTEX_PRIO_INHERIT | UMUTEX_PRIO_PROTECT)) {
@@ -2187,21 +2224,27 @@ do_cv_wait(struct thread *td, struct ucond *cv, struct umutex *m,
{
struct abs_timeout timo;
struct umtx_q *uq;
- uint32_t flags;
- uint32_t clockid;
+ uint32_t flags, clockid, hasw;
int error;
uq = td->td_umtxq;
- flags = fuword32(&cv->c_flags);
+ error = fueword32(&cv->c_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
error = umtx_key_get(cv, TYPE_CV, GET_SHARE(flags), &uq->uq_key);
if (error != 0)
return (error);
if ((wflags & CVWAIT_CLOCKID) != 0) {
- clockid = fuword32(&cv->c_clockid);
+ error = fueword32(&cv->c_clockid, &clockid);
+ if (error == -1) {
+ umtx_key_release(&uq->uq_key);
+ return (EFAULT);
+ }
if (clockid < CLOCK_REALTIME ||
clockid >= CLOCK_THREAD_CPUTIME_ID) {
/* hmm, only HW clock id will work. */
+ umtx_key_release(&uq->uq_key);
return (EINVAL);
}
} else {
@@ -2217,7 +2260,9 @@ do_cv_wait(struct thread *td, struct ucond *cv, struct umutex *m,
* Set c_has_waiters to 1 before releasing user mutex, also
* don't modify cache line when unnecessary.
*/
- if (fuword32(__DEVOLATILE(uint32_t *, &cv->c_has_waiters)) == 0)
+ error = fueword32(__DEVOLATILE(uint32_t *, &cv->c_has_waiters),
+ &hasw);
+ if (error == 0 && hasw == 0)
suword32(__DEVOLATILE(uint32_t *, &cv->c_has_waiters), 1);
umtxq_lock(&uq->uq_key);
@@ -2276,7 +2321,9 @@ do_cv_signal(struct thread *td, struct ucond *cv)
int error, cnt, nwake;
uint32_t flags;
- flags = fuword32(&cv->c_flags);
+ error = fueword32(&cv->c_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
if ((error = umtx_key_get(cv, TYPE_CV, GET_SHARE(flags), &key)) != 0)
return (error);
umtxq_lock(&key);
@@ -2287,6 +2334,8 @@ do_cv_signal(struct thread *td, struct ucond *cv)
umtxq_unlock(&key);
error = suword32(
__DEVOLATILE(uint32_t *, &cv->c_has_waiters), 0);
+ if (error == -1)
+ error = EFAULT;
umtxq_lock(&key);
}
umtxq_unbusy(&key);
@@ -2302,7 +2351,9 @@ do_cv_broadcast(struct thread *td, struct ucond *cv)
int error;
uint32_t flags;
- flags = fuword32(&cv->c_flags);
+ error = fueword32(&cv->c_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
if ((error = umtx_key_get(cv, TYPE_CV, GET_SHARE(flags), &key)) != 0)
return (error);
@@ -2312,6 +2363,8 @@ do_cv_broadcast(struct thread *td, struct ucond *cv)
umtxq_unlock(&key);
error = suword32(__DEVOLATILE(uint32_t *, &cv->c_has_waiters), 0);
+ if (error == -1)
+ error = EFAULT;
umtxq_lock(&key);
umtxq_unbusy(&key);
@@ -2329,10 +2382,12 @@ do_rw_rdlock(struct thread *td, struct urwlock *rwlock, long fflag, struct _umtx
uint32_t flags, wrflags;
int32_t state, oldstate;
int32_t blocked_readers;
- int error;
+ int error, rv;
uq = td->td_umtxq;
- flags = fuword32(&rwlock->rw_flags);
+ error = fueword32(&rwlock->rw_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
error = umtx_key_get(rwlock, TYPE_RWLOCK, GET_SHARE(flags), &uq->uq_key);
if (error != 0)
return (error);
@@ -2345,15 +2400,22 @@ do_rw_rdlock(struct thread *td, struct urwlock *rwlock, long fflag, struct _umtx
wrflags |= URWLOCK_WRITE_WAITERS;
for (;;) {
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *, &rwlock->rw_state),
+ &state);
+ if (rv == -1) {
+ umtx_key_release(&uq->uq_key);
+ return (EFAULT);
+ }
+
/* try to lock it */
while (!(state & wrflags)) {
if (__predict_false(URWLOCK_READER_COUNT(state) == URWLOCK_MAX_READERS)) {
umtx_key_release(&uq->uq_key);
return (EAGAIN);
}
- oldstate = casuword32(&rwlock->rw_state, state, state + 1);
- if (oldstate == -1) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state + 1);
+ if (rv == -1) {
umtx_key_release(&uq->uq_key);
return (EFAULT);
}
@@ -2379,12 +2441,17 @@ do_rw_rdlock(struct thread *td, struct urwlock *rwlock, long fflag, struct _umtx
* re-read the state, in case it changed between the try-lock above
* and the check below
*/
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *, &rwlock->rw_state),
+ &state);
+ if (rv == -1)
+ error = EFAULT;
/* set read contention bit */
- while ((state & wrflags) && !(state & URWLOCK_READ_WAITERS)) {
- oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_READ_WAITERS);
- if (oldstate == -1) {
+ while (error == 0 && (state & wrflags) &&
+ !(state & URWLOCK_READ_WAITERS)) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state | URWLOCK_READ_WAITERS);
+ if (rv == -1) {
error = EFAULT;
break;
}
@@ -2415,7 +2482,12 @@ do_rw_rdlock(struct thread *td, struct urwlock *rwlock, long fflag, struct _umtx
sleep:
/* contention bit is set, before sleeping, increase read waiter count */
- blocked_readers = fuword32(&rwlock->rw_blocked_readers);
+ rv = fueword32(&rwlock->rw_blocked_readers,
+ &blocked_readers);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
suword32(&rwlock->rw_blocked_readers, blocked_readers+1);
while (state & wrflags) {
@@ -2431,18 +2503,31 @@ sleep:
umtxq_unlock(&uq->uq_key);
if (error)
break;
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *,
+ &rwlock->rw_state), &state);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
}
/* decrease read waiter count, and may clear read contention bit */
- blocked_readers = fuword32(&rwlock->rw_blocked_readers);
+ error = fueword32(&rwlock->rw_blocked_readers,
+ &blocked_readers);
+ if (error == -1) {
+ error = EFAULT;
+ break;
+ }
suword32(&rwlock->rw_blocked_readers, blocked_readers-1);
if (blocked_readers == 1) {
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
- for (;;) {
- oldstate = casuword32(&rwlock->rw_state, state,
- state & ~URWLOCK_READ_WAITERS);
- if (oldstate == -1) {
+ rv = fueword32(__DEVOLATILE(int32_t *,
+ &rwlock->rw_state), &state);
+ if (rv == -1)
+ error = EFAULT;
+ while (error == 0) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state & ~URWLOCK_READ_WAITERS);
+ if (rv == -1) {
error = EFAULT;
break;
}
@@ -2450,8 +2535,6 @@ sleep:
break;
state = oldstate;
error = umtxq_check_susp(td);
- if (error != 0)
- break;
}
}
@@ -2476,10 +2559,12 @@ do_rw_wrlock(struct thread *td, struct urwlock *rwlock, struct _umtx_time *timeo
int32_t state, oldstate;
int32_t blocked_writers;
int32_t blocked_readers;
- int error;
+ int error, rv;
uq = td->td_umtxq;
- flags = fuword32(&rwlock->rw_flags);
+ error = fueword32(&rwlock->rw_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
error = umtx_key_get(rwlock, TYPE_RWLOCK, GET_SHARE(flags), &uq->uq_key);
if (error != 0)
return (error);
@@ -2489,10 +2574,16 @@ do_rw_wrlock(struct thread *td, struct urwlock *rwlock, struct _umtx_time *timeo
blocked_readers = 0;
for (;;) {
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *, &rwlock->rw_state),
+ &state);
+ if (rv == -1) {
+ umtx_key_release(&uq->uq_key);
+ return (EFAULT);
+ }
while (!(state & URWLOCK_WRITE_OWNER) && URWLOCK_READER_COUNT(state) == 0) {
- oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_WRITE_OWNER);
- if (oldstate == -1) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state | URWLOCK_WRITE_OWNER);
+ if (rv == -1) {
umtx_key_release(&uq->uq_key);
return (EFAULT);
}
@@ -2528,12 +2619,17 @@ do_rw_wrlock(struct thread *td, struct urwlock *rwlock, struct _umtx_time *timeo
* re-read the state, in case it changed between the try-lock above
* and the check below
*/
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *, &rwlock->rw_state),
+ &state);
+ if (rv == -1)
+ error = EFAULT;
- while (((state & URWLOCK_WRITE_OWNER) || URWLOCK_READER_COUNT(state) != 0) &&
- (state & URWLOCK_WRITE_WAITERS) == 0) {
- oldstate = casuword32(&rwlock->rw_state, state, state | URWLOCK_WRITE_WAITERS);
- if (oldstate == -1) {
+ while (error == 0 && ((state & URWLOCK_WRITE_OWNER) ||
+ URWLOCK_READER_COUNT(state) != 0) &&
+ (state & URWLOCK_WRITE_WAITERS) == 0) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state | URWLOCK_WRITE_WAITERS);
+ if (rv == -1) {
error = EFAULT;
break;
}
@@ -2561,7 +2657,12 @@ do_rw_wrlock(struct thread *td, struct urwlock *rwlock, struct _umtx_time *timeo
continue;
}
sleep:
- blocked_writers = fuword32(&rwlock->rw_blocked_writers);
+ rv = fueword32(&rwlock->rw_blocked_writers,
+ &blocked_writers);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
suword32(&rwlock->rw_blocked_writers, blocked_writers+1);
while ((state & URWLOCK_WRITE_OWNER) || URWLOCK_READER_COUNT(state) != 0) {
@@ -2577,17 +2678,34 @@ sleep:
umtxq_unlock(&uq->uq_key);
if (error)
break;
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *,
+ &rwlock->rw_state), &state);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
}
- blocked_writers = fuword32(&rwlock->rw_blocked_writers);
+ if (error != 0)
+ break;
+ rv = fueword32(&rwlock->rw_blocked_writers,
+ &blocked_writers);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
suword32(&rwlock->rw_blocked_writers, blocked_writers-1);
if (blocked_writers == 1) {
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ rv = fueword32(__DEVOLATILE(int32_t *,
+ &rwlock->rw_state), &state);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
for (;;) {
- oldstate = casuword32(&rwlock->rw_state, state,
- state & ~URWLOCK_WRITE_WAITERS);
- if (oldstate == -1) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state & ~URWLOCK_WRITE_WAITERS);
+ if (rv == -1) {
error = EFAULT;
break;
}
@@ -2603,7 +2721,12 @@ sleep:
if (error != 0)
break;
}
- blocked_readers = fuword32(&rwlock->rw_blocked_readers);
+ rv = fueword32(&rwlock->rw_blocked_readers,
+ &blocked_readers);
+ if (rv == -1) {
+ error = EFAULT;
+ break;
+ }
} else
blocked_readers = 0;
@@ -2624,20 +2747,24 @@ do_rw_unlock(struct thread *td, struct urwlock *rwlock)
struct umtx_q *uq;
uint32_t flags;
int32_t state, oldstate;
- int error, q, count;
+ int error, rv, q, count;
uq = td->td_umtxq;
- flags = fuword32(&rwlock->rw_flags);
+ error = fueword32(&rwlock->rw_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
error = umtx_key_get(rwlock, TYPE_RWLOCK, GET_SHARE(flags), &uq->uq_key);
if (error != 0)
return (error);
- state = fuword32(__DEVOLATILE(int32_t *, &rwlock->rw_state));
+ error = fueword32(__DEVOLATILE(int32_t *, &rwlock->rw_state), &state);
+ if (error == -1)
+ return (EFAULT);
if (state & URWLOCK_WRITE_OWNER) {
for (;;) {
- oldstate = casuword32(&rwlock->rw_state, state,
- state & ~URWLOCK_WRITE_OWNER);
- if (oldstate == -1) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state & ~URWLOCK_WRITE_OWNER);
+ if (rv == -1) {
error = EFAULT;
goto out;
}
@@ -2655,9 +2782,9 @@ do_rw_unlock(struct thread *td, struct urwlock *rwlock)
}
} else if (URWLOCK_READER_COUNT(state) != 0) {
for (;;) {
- oldstate = casuword32(&rwlock->rw_state, state,
- state - 1);
- if (oldstate == -1) {
+ rv = casueword32(&rwlock->rw_state, state,
+ &oldstate, state - 1);
+ if (rv == -1) {
error = EFAULT;
goto out;
}
@@ -2719,7 +2846,9 @@ do_sem_wait(struct thread *td, struct _usem *sem, struct _umtx_time *timeout)
int error;
uq = td->td_umtxq;
- flags = fuword32(&sem->_flags);
+ error = fueword32(&sem->_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
error = umtx_key_get(sem, TYPE_SEM, GET_SHARE(flags), &uq->uq_key);
if (error != 0)
return (error);
@@ -2732,14 +2861,14 @@ do_sem_wait(struct thread *td, struct _usem *sem, struct _umtx_time *timeout)
umtxq_insert(uq);
umtxq_unlock(&uq->uq_key);
casuword32(__DEVOLATILE(uint32_t *, &sem->_has_waiters), 0, 1);
- count = fuword32(__DEVOLATILE(uint32_t *, &sem->_count));
- if (count != 0) {
+ error = fueword32(__DEVOLATILE(uint32_t *, &sem->_count), &count);
+ if (error == -1 || count != 0) {
umtxq_lock(&uq->uq_key);
umtxq_unbusy(&uq->uq_key);
umtxq_remove(uq);
umtxq_unlock(&uq->uq_key);
umtx_key_release(&uq->uq_key);
- return (0);
+ return (error == -1 ? EFAULT : 0);
}
umtxq_lock(&uq->uq_key);
umtxq_unbusy(&uq->uq_key);
@@ -2770,7 +2899,9 @@ do_sem_wake(struct thread *td, struct _usem *sem)
int error, cnt;
uint32_t flags;
- flags = fuword32(&sem->_flags);
+ error = fueword32(&sem->_flags, &flags);
+ if (error == -1)
+ return (EFAULT);
if ((error = umtx_key_get(sem, TYPE_SEM, GET_SHARE(flags), &key)) != 0)
return (error);
umtxq_lock(&key);
@@ -2788,6 +2919,8 @@ do_sem_wake(struct thread *td, struct _usem *sem)
error = suword32(
__DEVOLATILE(uint32_t *, &sem->_has_waiters), 0);
umtxq_lock(&key);
+ if (error == -1)
+ error = EFAULT;
}
}
umtxq_unbusy(&key);
diff --git a/sys/kern/subr_uio.c b/sys/kern/subr_uio.c
index f2e6e32..f2bbb0c 100644
--- a/sys/kern/subr_uio.c
+++ b/sys/kern/subr_uio.c
@@ -7,6 +7,11 @@
* Co. or Unix System Laboratories, Inc. and are reproduced herein with
* the permission of UNIX System Laboratories, Inc.
*
+ * Copyright (c) 2014 The FreeBSD Foundation
+ *
+ * Portions of this software were developed by Konstantin Belousov
+ * under sponsorship from the FreeBSD Foundation.
+ *
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
@@ -438,3 +443,128 @@ copyout_unmap(struct thread *td, vm_offset_t addr, size_t sz)
return (0);
}
+
+#ifdef NO_FUEWORD
+/*
+ * XXXKIB The temporal implementation of fue*() functions which do not
+ * handle usermode -1 properly, mixing it with the fault code. Keep
+ * this until MD code is written. Currently sparc64, mips and arm do
+ * not have proper implementation.
+ */
+
+int
+fueword(const void *base, long *val)
+{
+ long res;
+
+ res = fuword(base);
+ if (res == -1)
+ return (-1);
+ *val = res;
+ return (0);
+}
+
+int
+fueword32(const void *base, int32_t *val)
+{
+ int32_t res;
+
+ res = fuword32(base);
+ if (res == -1)
+ return (-1);
+ *val = res;
+ return (0);
+}
+
+#ifdef _LP64
+int
+fueword64(const void *base, int64_t *val)
+{
+ int32_t res;
+
+ res = fuword64(base);
+ if (res == -1)
+ return (-1);
+ *val = res;
+ return (0);
+}
+#endif
+
+int
+casueword32(volatile uint32_t *base, uint32_t oldval, uint32_t *oldvalp,
+ uint32_t newval)
+{
+ int32_t ov;
+
+ ov = casuword32(base, oldval, newval);
+ if (ov == -1)
+ return (-1);
+ *oldvalp = ov;
+ return (0);
+}
+
+int
+casueword(volatile u_long *p, u_long oldval, u_long *oldvalp, u_long newval)
+{
+ u_long ov;
+
+ ov = casuword(p, oldval, newval);
+ if (ov == -1)
+ return (-1);
+ *oldvalp = ov;
+ return (0);
+}
+#else /* NO_FUEWORD */
+int32_t
+fuword32(const void *addr)
+{
+ int rv;
+ int32_t val;
+
+ rv = fueword32(addr, &val);
+ return (rv == -1 ? -1 : val);
+}
+
+#ifdef _LP64
+int64_t
+fuword64(const void *addr)
+{
+ int rv;
+ int64_t val;
+
+ rv = fueword64(addr, &val);
+ return (rv == -1 ? -1 : val);
+}
+#endif /* _LP64 */
+
+long
+fuword(const void *addr)
+{
+ long val;
+ int rv;
+
+ rv = fueword(addr, &val);
+ return (rv == -1 ? -1 : val);
+}
+
+uint32_t
+casuword32(volatile uint32_t *addr, uint32_t old, uint32_t new)
+{
+ int rv;
+ uint32_t val;
+
+ rv = casueword32(addr, old, &val, new);
+ return (rv == -1 ? -1 : val);
+}
+
+u_long
+casuword(volatile u_long *addr, u_long old, u_long new)
+{
+ int rv;
+ u_long val;
+
+ rv = casueword(addr, old, &val, new);
+ return (rv == -1 ? -1 : val);
+}
+
+#endif /* NO_FUEWORD */
diff --git a/sys/kern/vfs_acl.c b/sys/kern/vfs_acl.c
index 93626fb..e9361e5 100644
--- a/sys/kern/vfs_acl.c
+++ b/sys/kern/vfs_acl.c
@@ -148,6 +148,7 @@ acl_copyin(void *user_acl, struct acl *kernel_acl, acl_type_t type)
static int
acl_copyout(struct acl *kernel_acl, void *user_acl, acl_type_t type)
{
+ uint32_t am;
int error;
struct oldacl old;
@@ -162,8 +163,11 @@ acl_copyout(struct acl *kernel_acl, void *user_acl, acl_type_t type)
break;
default:
- if (fuword32((char *)user_acl +
- offsetof(struct acl, acl_maxcnt)) != ACL_MAX_ENTRIES)
+ error = fueword32((char *)user_acl +
+ offsetof(struct acl, acl_maxcnt), &am);
+ if (error == -1)
+ return (EFAULT);
+ if (am != ACL_MAX_ENTRIES)
return (EINVAL);
error = copyout(kernel_acl, user_acl, sizeof(*kernel_acl));
diff --git a/sys/mips/include/param.h b/sys/mips/include/param.h
index 2d1d7f1..90f3e6f 100644
--- a/sys/mips/include/param.h
+++ b/sys/mips/include/param.h
@@ -178,4 +178,8 @@
#define pgtok(x) ((x) * (PAGE_SIZE / 1024))
+#ifdef _KERNEL
+#define NO_FUEWORD 1
+#endif
+
#endif /* !_MIPS_INCLUDE_PARAM_H_ */
diff --git a/sys/net/if_spppsubr.c b/sys/net/if_spppsubr.c
index 9dc55c5..c0f8e39 100644
--- a/sys/net/if_spppsubr.c
+++ b/sys/net/if_spppsubr.c
@@ -5060,7 +5060,8 @@ sppp_params(struct sppp *sp, u_long cmd, void *data)
* Check the cmd word first before attempting to fetch all the
* data.
*/
- if ((subcmd = fuword(ifr->ifr_data)) == -1) {
+ rv = fueword(ifr->ifr_data, &subcmd);
+ if (rv == -1) {
rv = EFAULT;
goto quit;
}
diff --git a/sys/powerpc/powerpc/copyinout.c b/sys/powerpc/powerpc/copyinout.c
index dcfab80..a337c8b 100644
--- a/sys/powerpc/powerpc/copyinout.c
+++ b/sys/powerpc/powerpc/copyinout.c
@@ -405,14 +405,13 @@ fubyte(const void *addr)
return (val);
}
-#ifdef __powerpc64__
-int32_t
-fuword32(const void *addr)
+int
+fuword16(const void *addr)
{
struct thread *td;
pmap_t pm;
faultbuf env;
- int32_t *p, val;
+ uint16_t *p, val;
td = curthread;
pm = &td->td_proc->p_vmspace->vm_pmap;
@@ -432,15 +431,14 @@ fuword32(const void *addr)
td->td_pcb->pcb_onfault = NULL;
return (val);
}
-#endif
-long
-fuword(const void *addr)
+int
+fueword32(const void *addr, int32_t *val)
{
struct thread *td;
pmap_t pm;
faultbuf env;
- long *p, val;
+ int32_t *p;
td = curthread;
pm = &td->td_proc->p_vmspace->vm_pmap;
@@ -455,22 +453,71 @@ fuword(const void *addr)
return (-1);
}
- val = *p;
+ *val = *p;
td->td_pcb->pcb_onfault = NULL;
- return (val);
+ return (0);
}
-#ifndef __powerpc64__
-int32_t
-fuword32(const void *addr)
+#ifdef __powerpc64__
+int
+fueword64(const void *addr, int64_t *val)
{
- return ((int32_t)fuword(addr));
+ struct thread *td;
+ pmap_t pm;
+ faultbuf env;
+ int64_t *p;
+
+ td = curthread;
+ pm = &td->td_proc->p_vmspace->vm_pmap;
+
+ if (setfault(env)) {
+ td->td_pcb->pcb_onfault = NULL;
+ return (-1);
+ }
+
+ if (map_user_ptr(pm, addr, (void **)&p, sizeof(*p), NULL)) {
+ td->td_pcb->pcb_onfault = NULL;
+ return (-1);
+ }
+
+ *val = *p;
+
+ td->td_pcb->pcb_onfault = NULL;
+ return (0);
}
#endif
-uint32_t
-casuword32(volatile uint32_t *addr, uint32_t old, uint32_t new)
+int
+fueword(const void *addr, long *val)
+{
+ struct thread *td;
+ pmap_t pm;
+ faultbuf env;
+ long *p;
+
+ td = curthread;
+ pm = &td->td_proc->p_vmspace->vm_pmap;
+
+ if (setfault(env)) {
+ td->td_pcb->pcb_onfault = NULL;
+ return (-1);
+ }
+
+ if (map_user_ptr(pm, addr, (void **)&p, sizeof(*p), NULL)) {
+ td->td_pcb->pcb_onfault = NULL;
+ return (-1);
+ }
+
+ *val = *p;
+
+ td->td_pcb->pcb_onfault = NULL;
+ return (0);
+}
+
+int
+casueword32(volatile uint32_t *addr, uint32_t old, uint32_t *oldvalp,
+ uint32_t new)
{
struct thread *td;
pmap_t pm;
@@ -507,18 +554,21 @@ casuword32(volatile uint32_t *addr, uint32_t old, uint32_t new)
td->td_pcb->pcb_onfault = NULL;
- return (val);
+ *oldvalp = val;
+ return (0);
}
#ifndef __powerpc64__
-u_long
-casuword(volatile u_long *addr, u_long old, u_long new)
+int
+casueword(volatile u_long *addr, u_long old, u_long *oldvalp, u_long new)
{
- return (casuword32((volatile uint32_t *)addr, old, new));
+
+ return (casueword32((volatile uint32_t *)addr, old,
+ (uint32_t *)oldvalp, new));
}
#else
-u_long
-casuword(volatile u_long *addr, u_long old, u_long new)
+int
+casueword(volatile u_long *addr, u_long old, u_long *oldvalp, u_long new)
{
struct thread *td;
pmap_t pm;
@@ -555,7 +605,7 @@ casuword(volatile u_long *addr, u_long old, u_long new)
td->td_pcb->pcb_onfault = NULL;
- return (val);
+ *oldvalp = val;
+ return (0);
}
#endif
-
diff --git a/sys/sparc64/include/param.h b/sys/sparc64/include/param.h
index e59f2c4..46bacae 100644
--- a/sys/sparc64/include/param.h
+++ b/sys/sparc64/include/param.h
@@ -146,4 +146,8 @@
#define pgtok(x) ((unsigned long)(x) * (PAGE_SIZE / 1024))
+#ifdef _KERNEL
+#define NO_FUEWORD 1
+#endif
+
#endif /* !_SPARC64_INCLUDE_PARAM_H_ */
diff --git a/sys/sys/systm.h b/sys/sys/systm.h
index f4eae57..6e5ee61 100644
--- a/sys/sys/systm.h
+++ b/sys/sys/systm.h
@@ -254,16 +254,23 @@ int copyout_nofault(const void * __restrict kaddr, void * __restrict udaddr,
int fubyte(const void *base);
long fuword(const void *base);
-int fuword16(void *base);
+int fuword16(const void *base);
int32_t fuword32(const void *base);
int64_t fuword64(const void *base);
+int fueword(const void *base, long *val);
+int fueword32(const void *base, int32_t *val);
+int fueword64(const void *base, int64_t *val);
int subyte(void *base, int byte);
int suword(void *base, long word);
int suword16(void *base, int word);
int suword32(void *base, int32_t word);
int suword64(void *base, int64_t word);
uint32_t casuword32(volatile uint32_t *base, uint32_t oldval, uint32_t newval);
-u_long casuword(volatile u_long *p, u_long oldval, u_long newval);
+u_long casuword(volatile u_long *p, u_long oldval, u_long newval);
+int casueword32(volatile uint32_t *base, uint32_t oldval, uint32_t *oldvalp,
+ uint32_t newval);
+int casueword(volatile u_long *p, u_long oldval, u_long *oldvalp,
+ u_long newval);
void realitexpire(void *);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141021162306.GE1877>