copystr(9): Move to deprecate [2/2]

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.

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
Differential Revision:	https://reviews.freebsd.org/D24672

sys/amd64/amd64/support.S

@@ -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
  */

sys/arm/arm/copystr.S

@@ -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}
 

sys/arm64/arm64/copystr.c

@@ -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);
-}
-

sys/fs/fuse/fuse_vfsops.c

@@ -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;
@@ -432,8 +430,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);
+	memset(mp->mnt_stat.f_mntfromname, 0, MNAMELEN);
-	bzero(mp->mnt_stat.f_mntfromname + len, MNAMELEN - len);
+	strlcpy(mp->mnt_stat.f_mntfromname, fspec, MNAMELEN);
 	mp->mnt_iosize_max = MAXPHYS;
 
 	/* Now handshaking with daemon */

sys/fs/unionfs/union_vfsops.c

@@ -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;
+	snprintf(mp->mnt_stat.f_mntfromname, MNAMELEN, "<%s>:%s",
-	tmp = mp->mnt_stat.f_mntfromname;
+	    below ? "below" : "above", target);
-	copystr((below ? "<below>:" : "<above>:"), tmp, len, &done);
-	len -= done - 1;
-	tmp += done - 1;
-	copystr(target, tmp, len, NULL);
 
 	UNIONFSDEBUG("unionfs_mount: from %s, on %s\n",
 	    mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntonname);

sys/i386/i386/support.s

@@ -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

sys/kern/subr_csan.c

@@ -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)
 {

sys/mips/mips/support.S

@@ -105,12 +105,22 @@
 	.text
 
 /*
- * int copystr(void *kfaddr, void *kdaddr, size_t maxlen, size_t *lencopied)
+ * Copy a null terminated string from the user address space into
- * Copy a NIL-terminated string, at most maxlen characters long.  Return the
+ * the kernel address space.
- * number of characters copied (including the NIL) in *lencopied.  If the
+ *
- * string is too long, return ENAMETOOLONG; else return 0.
+ *	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)
 
-
+__copyinstr_err:
-/*
+	j		ra
- * Copy a null terminated string from the user address space into
+	li		v0, EFAULT
- * 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
 END(copyinstr)
 
 /*

sys/powerpc/powerpc/copystr.c

@@ -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;
-}

sys/riscv/riscv/copystr.c

@@ -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);
-}

sys/sys/systm.h

@@ -362,9 +362,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,
+#define	copystr(src, dst, len, outlen)	({			\
-	    void * _Nonnull __restrict kdaddr, size_t len,
+	size_t __r, __len, *__outlen;				\
-	    size_t * __restrict lencopied);
+								\
+	__len = (len);						\
+	__outlen = (outlen);					\
+	__r = strlcpy((dst), (src), __len);			\
+	if (__outlen != NULL)					\
+		*__outlen = ((__r >= __len) ? __len : __r);	\
+	((__r >= __len) ? ENAMETOOLONG : 0);			\
+})
+
 int	copyinstr(const void * __restrict udaddr,
 	    void * _Nonnull __restrict kaddr, size_t len,
 	    size_t * __restrict lencopied);
@@ -378,11 +386,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))

tools/coccinelle/copystr9.cocci

@@ -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);
+)