svn commit: r360944 - in head: sys/amd64/amd64 sys/arm/arm sys/arm64/arm64 sys/fs/fuse sys/fs/unionfs sys/i386/i386 sys/kern sys/mips/mips sys/powerpc/powerpc sys/riscv/riscv sys/sys tools/coccinelle
Conrad Meyer
cem at FreeBSD.org
Mon May 11 22:57:24 UTC 2020
Author: cem
Date: Mon May 11 22:57:21 2020
New Revision: 360944
URL: https://svnweb.freebsd.org/changeset/base/360944
Log:
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
Added:
head/tools/coccinelle/
head/tools/coccinelle/copystr9.cocci (contents, props changed)
Deleted:
head/sys/arm64/arm64/copystr.c
head/sys/powerpc/powerpc/copystr.c
head/sys/riscv/riscv/copystr.c
Modified:
head/sys/amd64/amd64/support.S
head/sys/arm/arm/copystr.S
head/sys/fs/fuse/fuse_vfsops.c
head/sys/fs/unionfs/union_vfsops.c
head/sys/i386/i386/support.s
head/sys/kern/subr_csan.c
head/sys/mips/mips/support.S
head/sys/sys/systm.h
Modified: head/sys/amd64/amd64/support.S
==============================================================================
--- head/sys/amd64/amd64/support.S Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/amd64/amd64/support.S Mon May 11 22:57:21 2020 (r360944)
@@ -1417,43 +1417,6 @@ copyinstr_toolong:
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
*/
/* void lgdt(struct region_descriptor *rdp); */
Modified: head/sys/arm/arm/copystr.S
==============================================================================
--- head/sys/arm/arm/copystr.S Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/arm/arm/copystr.S Mon May 11 22:57:21 2020 (r360944)
@@ -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}
Modified: head/sys/fs/fuse/fuse_vfsops.c
==============================================================================
--- head/sys/fs/fuse/fuse_vfsops.c Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/fs/fuse/fuse_vfsops.c Mon May 11 22:57:21 2020 (r360944)
@@ -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);
- 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 */
Modified: head/sys/fs/unionfs/union_vfsops.c
==============================================================================
--- head/sys/fs/unionfs/union_vfsops.c Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/fs/unionfs/union_vfsops.c Mon May 11 22:57:21 2020 (r360944)
@@ -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);
Modified: head/sys/i386/i386/support.s
==============================================================================
--- head/sys/i386/i386/support.s Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/i386/i386/support.s Mon May 11 22:57:21 2020 (r360944)
@@ -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
Modified: head/sys/kern/subr_csan.c
==============================================================================
--- head/sys/kern/subr_csan.c Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/kern/subr_csan.c Mon May 11 22:57:21 2020 (r360944)
@@ -350,19 +350,11 @@ 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)
Modified: head/sys/mips/mips/support.S
==============================================================================
--- head/sys/mips/mips/support.S Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/mips/mips/support.S Mon May 11 22:57:21 2020 (r360944)
@@ -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)
/*
Modified: head/sys/sys/systm.h
==============================================================================
--- head/sys/sys/systm.h Mon May 11 22:48:00 2020 (r360943)
+++ head/sys/sys/systm.h Mon May 11 22:57:21 2020 (r360944)
@@ -362,9 +362,17 @@ void *memcpy_early(void * _Nonnull to, const void * _N
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); \
+ ((__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 k
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))
Added: head/tools/coccinelle/copystr9.cocci
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ head/tools/coccinelle/copystr9.cocci Mon May 11 22:57:21 2020 (r360944)
@@ -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);
+)
More information about the svn-src-all
mailing list