copystr(9): Move to deprecate (attempt #2)

This reapplies logical r360944 and r360946 (reverting r360955), with fixed
copystr() stand-in replacement macro.  Eventually the goal is to convert
consumers and kill the macro, but for a first step it helps if the macro is
correct.

Prior commit message:

Unlike the other copy*() functions, it does not serve to copy from one
address space to another or protect against potential faults.  It's just
an older incarnation of the now-more-common strlcpy().

Add a coccinelle script to tools/ which can be used to mechanically
convert existing instances where replacement with strlcpy is trivial.
In the two cases which matched, fuse_vfsops.c and union_vfsops.c, the
code was further refactored manually to simplify.

Replace the declaration of copystr() in systm.h with a small macro
wrapper around strlcpy (with correction from brooks@ -- thanks).

Remove N redundant MI implementations of copystr.  For MIPS, this
entailed inlining the assembler copystr into the only consumer,
copyinstr, and making the latter a leaf function.

Reviewed by:		jhb (earlier version)
Discussed with:		brooks (thanks!)
Differential Revision:	https://reviews.freebsd.org/D24672
This commit is contained in:
Conrad Meyer 2020-05-25 16:40:48 +00:00
parent 9085d7d6b8
commit 852c303b61
15 changed files with 75 additions and 362 deletions

View File

@ -1416,43 +1416,6 @@ copyinstr_toolong:
movl $ENAMETOOLONG,%eax
jmp cpystrflt_x
/*
* copystr(from, to, maxlen, int *lencopied)
* %rdi, %rsi, %rdx, %rcx
*/
ENTRY(copystr)
PUSH_FRAME_POINTER
movq %rdx,%r8 /* %r8 = maxlen */
incq %rdx
1:
decq %rdx
jz 4f
movb (%rdi),%al
movb %al,(%rsi)
incq %rsi
incq %rdi
testb %al,%al
jnz 1b
/* Success -- 0 byte reached */
decq %rdx
xorl %eax,%eax
2:
testq %rcx,%rcx
jz 3f
/* set *lencopied and return %rax */
subq %rdx,%r8
movq %r8,(%rcx)
3:
POP_FRAME_POINTER
ret
4:
/* rdx is zero -- return ENAMETOOLONG */
movl $ENAMETOOLONG,%eax
jmp 2b
END(copystr)
/*
* Handling of special amd64 registers and descriptor tables etc
*/

View File

@ -60,39 +60,6 @@ __FBSDID("$FreeBSD$");
ldr tmp, .Lpcb
#endif
/*
* r0 - from
* r1 - to
* r2 - maxlens
* r3 - lencopied
*
* Copy string from r0 to r1
*/
ENTRY(copystr)
stmfd sp!, {r4-r5} /* stack is 8 byte aligned */
teq r2, #0x00000000
mov r5, #0x00000000
moveq r0, #ENAMETOOLONG
beq 2f
1: ldrb r4, [r0], #0x0001
add r5, r5, #0x00000001
teq r4, #0x00000000
strb r4, [r1], #0x0001
teqne r5, r2
bne 1b
teq r4, #0x00000000
moveq r0, #0x00000000
movne r0, #ENAMETOOLONG
2: teq r3, #0x00000000
strne r5, [r3]
ldmfd sp!, {r4-r5} /* stack is 8 byte aligned */
RET
END(copystr)
#define SAVE_REGS stmfd sp!, {r4-r6}
#define RESTORE_REGS ldmfd sp!, {r4-r6}

View File

@ -1,61 +0,0 @@
/*-
* Copyright (c) 2014 Andrew Turner
* All rights reserved.
*
* 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 AUTHOR 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 AUTHOR 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.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
int
(copystr)(const void * __restrict kfaddr, void * __restrict kdaddr, size_t len,
size_t * __restrict lencopied)
{
const char *src;
size_t pos;
char *dst;
int error;
error = ENAMETOOLONG;
src = kfaddr;
dst = kdaddr;
for (pos = 0; pos < len; pos++) {
dst[pos] = src[pos];
if (src[pos] == '\0') {
/* Increment pos to hold the number of bytes copied */
pos++;
error = 0;
break;
}
}
if (lencopied != NULL)
*lencopied = pos;
return (error);
}

View File

@ -134,7 +134,6 @@ arm64/arm64/busdma_machdep.c standard
arm64/arm64/bzero.S standard
arm64/arm64/clock.c standard
arm64/arm64/copyinout.S standard
arm64/arm64/copystr.c standard
arm64/arm64/cpu_errata.c standard
arm64/arm64/cpufunc_asm.S standard
arm64/arm64/db_disasm.c optional ddb

View File

@ -241,7 +241,6 @@ powerpc/powerpc/bus_machdep.c standard
powerpc/powerpc/busdma_machdep.c standard
powerpc/powerpc/clock.c standard
powerpc/powerpc/copyinout.c standard
powerpc/powerpc/copystr.c standard
powerpc/powerpc/cpu.c standard
powerpc/powerpc/cpu_subr64.S optional powerpc64
powerpc/powerpc/db_disasm.c optional ddb

View File

@ -37,7 +37,6 @@ riscv/riscv/busdma_bounce.c standard
riscv/riscv/busdma_machdep.c standard
riscv/riscv/clock.c standard
riscv/riscv/copyinout.S standard
riscv/riscv/copystr.c standard
riscv/riscv/cpufunc_asm.S standard
riscv/riscv/db_disasm.c optional ddb
riscv/riscv/db_interface.c optional ddb

View File

@ -303,8 +303,6 @@ fuse_vfsop_mount(struct mount *mp)
int daemon_timeout;
int fd;
size_t len;
struct cdev *fdev;
struct fuse_data *data = NULL;
struct thread *td;
@ -437,8 +435,8 @@ fuse_vfsop_mount(struct mount *mp)
strlcat(mp->mnt_stat.f_fstypename, ".", MFSNAMELEN);
strlcat(mp->mnt_stat.f_fstypename, subtype, MFSNAMELEN);
}
copystr(fspec, mp->mnt_stat.f_mntfromname, MNAMELEN - 1, &len);
bzero(mp->mnt_stat.f_mntfromname + len, MNAMELEN - len);
memset(mp->mnt_stat.f_mntfromname, 0, MNAMELEN);
strlcpy(mp->mnt_stat.f_mntfromname, fspec, MNAMELEN);
mp->mnt_iosize_max = MAXPHYS;
/* Now handshaking with daemon */

View File

@ -83,7 +83,6 @@ unionfs_domount(struct mount *mp)
char *tmp;
char *ep;
int len;
size_t done;
int below;
uid_t uid;
gid_t gid;
@ -304,12 +303,8 @@ unionfs_domount(struct mount *mp)
*/
vfs_getnewfsid(mp);
len = MNAMELEN - 1;
tmp = mp->mnt_stat.f_mntfromname;
copystr((below ? "<below>:" : "<above>:"), tmp, len, &done);
len -= done - 1;
tmp += done - 1;
copystr(target, tmp, len, NULL);
snprintf(mp->mnt_stat.f_mntfromname, MNAMELEN, "<%s>:%s",
below ? "below" : "above", target);
UNIONFSDEBUG("unionfs_mount: from %s, on %s\n",
mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntonname);

View File

@ -233,47 +233,6 @@ ENTRY(memcpy)
ret
END(memcpy)
/*
* copystr(from, to, maxlen, int *lencopied) - MP SAFE
*/
ENTRY(copystr)
pushl %esi
pushl %edi
movl 12(%esp),%esi /* %esi = from */
movl 16(%esp),%edi /* %edi = to */
movl 20(%esp),%edx /* %edx = maxlen */
incl %edx
1:
decl %edx
jz 4f
lodsb
stosb
orb %al,%al
jnz 1b
/* Success -- 0 byte reached */
decl %edx
xorl %eax,%eax
jmp 6f
4:
/* edx is zero -- return ENAMETOOLONG */
movl $ENAMETOOLONG,%eax
6:
/* set *lencopied and return %eax */
movl 20(%esp),%ecx
subl %edx,%ecx
movl 24(%esp),%edx
testl %edx,%edx
jz 7f
movl %ecx,(%edx)
7:
popl %edi
popl %esi
ret
END(copystr)
ENTRY(bcmp)
pushl %edi
pushl %esi

View File

@ -350,20 +350,12 @@ kcsan_strlen(const char *str)
return (s - str);
}
#undef copystr
#undef copyin
#undef copyin_nofault
#undef copyinstr
#undef copyout
#undef copyout_nofault
int
kcsan_copystr(const void *kfaddr, void *kdaddr, size_t len, size_t *done)
{
kcsan_access((uintptr_t)kdaddr, len, true, false, __RET_ADDR);
return copystr(kfaddr, kdaddr, len, done);
}
int
kcsan_copyin(const void *uaddr, void *kaddr, size_t len)
{

View File

@ -105,12 +105,22 @@
.text
/*
* int copystr(void *kfaddr, void *kdaddr, size_t maxlen, size_t *lencopied)
* Copy a NIL-terminated string, at most maxlen characters long. Return the
* number of characters copied (including the NIL) in *lencopied. If the
* string is too long, return ENAMETOOLONG; else return 0.
* Copy a null terminated string from the user address space into
* the kernel address space.
*
* copyinstr(fromaddr, toaddr, maxlength, &lencopied)
* caddr_t fromaddr;
* caddr_t toaddr;
* u_int maxlength;
* u_int *lencopied;
*/
LEAF(copystr)
LEAF(copyinstr)
PTR_LA v0, __copyinstr_err
blt a0, zero, __copyinstr_err # make sure address is in user space
GET_CPU_PCPU(v1)
PTR_L v1, PC_CURPCB(v1)
PTR_S v0, U_PCB_ONFAULT(v1)
move t0, a2
beq a2, zero, 4f
1:
@ -128,37 +138,14 @@ LEAF(copystr)
PTR_SUBU a2, t0, a2 # if the 4th arg was non-NULL
PTR_S a2, 0(a3)
3:
j ra # v0 is 0 or ENAMETOOLONG
PTR_S zero, U_PCB_ONFAULT(v1)
j ra
nop
END(copystr)
/*
* Copy a null terminated string from the user address space into
* the kernel address space.
*
* copyinstr(fromaddr, toaddr, maxlength, &lencopied)
* caddr_t fromaddr;
* caddr_t toaddr;
* u_int maxlength;
* u_int *lencopied;
*/
NESTED(copyinstr, CALLFRAME_SIZ, ra)
PTR_SUBU sp, sp, CALLFRAME_SIZ
.mask 0x80000000, (CALLFRAME_RA - CALLFRAME_SIZ)
PTR_LA v0, copyerr
blt a0, zero, _C_LABEL(copyerr) # make sure address is in user space
REG_S ra, CALLFRAME_RA(sp)
GET_CPU_PCPU(v1)
PTR_L v1, PC_CURPCB(v1)
jal _C_LABEL(copystr)
PTR_S v0, U_PCB_ONFAULT(v1)
REG_L ra, CALLFRAME_RA(sp)
GET_CPU_PCPU(v1)
PTR_L v1, PC_CURPCB(v1)
PTR_S zero, U_PCB_ONFAULT(v1)
j ra
PTR_ADDU sp, sp, CALLFRAME_SIZ
__copyinstr_err:
j ra
li v0, EFAULT
END(copyinstr)
/*

View File

@ -1,70 +0,0 @@
/*-
* SPDX-License-Identifier: BSD-4-Clause
*
* Copyright (C) 1995 Wolfgang Solfrank.
* Copyright (C) 1995 TooLs GmbH.
* All rights reserved.
*
* 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.
* 3. All advertising materials mentioning features or use of this software
* must display the following acknowledgement:
* This product includes software developed by TooLs GmbH.
* 4. The name of TooLs GmbH may not be used to endorse or promote products
* derived from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY TOOLS GMBH ``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 TOOLS GMBH 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.
*
* $NetBSD: copystr.c,v 1.3 2000/06/08 06:47:17 kleink Exp $
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/errno.h>
#include <sys/systm.h>
/*
* Emulate copyinstr.
*/
int
copystr(kfaddr, kdaddr, len, done)
const void *kfaddr;
void *kdaddr;
size_t len;
size_t *done;
{
const u_char *kfp = kfaddr;
u_char *kdp = kdaddr;
size_t l;
int rv;
rv = ENAMETOOLONG;
for (l = 0; len-- > 0; l++) {
if (!(*kdp++ = *kfp++)) {
l++;
rv = 0;
break;
}
}
if (done != NULL) {
*done = l;
}
return rv;
}

View File

@ -1,59 +0,0 @@
/*-
* Copyright (c) 2014 Andrew Turner
* All rights reserved.
*
* 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 AUTHOR 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 AUTHOR 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.
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
int
copystr(const void * __restrict kfaddr, void * __restrict kdaddr, size_t len,
size_t * __restrict lencopied)
{
const char *src;
size_t pos;
char *dst;
int error;
error = ENAMETOOLONG;
src = kfaddr;
dst = kdaddr;
for (pos = 0; pos < len; pos++) {
dst[pos] = src[pos];
if (src[pos] == '\0') {
/* Increment pos to hold the number of bytes copied */
pos++;
error = 0;
break;
}
}
if (lencopied != NULL)
*lencopied = pos;
return (error);
}

View File

@ -366,9 +366,17 @@ void *memcpy_early(void * _Nonnull to, const void * _Nonnull from, size_t len);
void *memmove_early(void * _Nonnull dest, const void * _Nonnull src, size_t n);
#define bcopy_early(from, to, len) memmove_early((to), (from), (len))
int copystr(const void * _Nonnull __restrict kfaddr,
void * _Nonnull __restrict kdaddr, size_t len,
size_t * __restrict lencopied);
#define copystr(src, dst, len, outlen) ({ \
size_t __r, __len, *__outlen; \
\
__len = (len); \
__outlen = (outlen); \
__r = strlcpy((dst), (src), __len); \
if (__outlen != NULL) \
*__outlen = ((__r >= __len) ? __len : __r + 1); \
((__r >= __len) ? ENAMETOOLONG : 0); \
})
int copyinstr(const void * __restrict udaddr,
void * _Nonnull __restrict kaddr, size_t len,
size_t * __restrict lencopied);
@ -382,11 +390,9 @@ int copyout_nofault(const void * _Nonnull __restrict kaddr,
void * __restrict udaddr, size_t len);
#ifdef KCSAN
int kcsan_copystr(const void *, void *, size_t, size_t *);
int kcsan_copyin(const void *, void *, size_t);
int kcsan_copyinstr(const void *, void *, size_t, size_t *);
int kcsan_copyout(const void *, void *, size_t);
#define copystr(kf, k, l, lc) kcsan_copystr((kf), (k), (l), (lc))
#define copyin(u, k, l) kcsan_copyin((u), (k), (l))
#define copyinstr(u, k, l, lc) kcsan_copyinstr((u), (k), (l), (lc))
#define copyout(k, u, l) kcsan_copyout((k), (u), (l))

View File

@ -0,0 +1,39 @@
@ nostorederror_nostoredlen @
expression __src, __dst, __len;
statement S1;
@@
S1
-copystr(__src, __dst, __len, NULL);
+strlcpy(__dst, __src, __len);
@ ifcondition_nostoredlen @
expression __src, __dst, __len;
statement S1;
@@
if (
(
-copystr(__src, __dst, __len, NULL) == ENAMETOOLONG
|
-copystr(__src, __dst, __len, NULL) != 0
|
-copystr(__src, __dst, __len, NULL)
)
+strlcpy(__dst, __src, __len) >= __len
) S1
@ nostorederror_storedlen1 @
expression __src, __dst, __len;
identifier __done;
statement S1;
@@
S1
(
-copystr(__src, __dst, __len, &__done);
+__done = strlcpy(__dst, __src, __len);
+__done = MIN(__done, __len);
|
-copystr(__src, __dst, __len, __done);
+ *__done = strlcpy(__dst, __src, __len);
+ *__done = MIN(*__done, __len);
)