Author: cem Date: Mon May 25 16:40:48 2020 New Revision: 361466 URL: https://svnweb.freebsd.org/changeset/base/361466
Log: 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 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/conf/files.arm64 head/sys/conf/files.powerpc head/sys/conf/files.riscv 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 25 16:06:30 2020 (r361465) +++ head/sys/amd64/amd64/support.S Mon May 25 16:40:48 2020 (r361466) @@ -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 25 16:06:30 2020 (r361465) +++ head/sys/arm/arm/copystr.S Mon May 25 16:40:48 2020 (r361466) @@ -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/conf/files.arm64 ============================================================================== --- head/sys/conf/files.arm64 Mon May 25 16:06:30 2020 (r361465) +++ head/sys/conf/files.arm64 Mon May 25 16:40:48 2020 (r361466) @@ -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 Modified: head/sys/conf/files.powerpc ============================================================================== --- head/sys/conf/files.powerpc Mon May 25 16:06:30 2020 (r361465) +++ head/sys/conf/files.powerpc Mon May 25 16:40:48 2020 (r361466) @@ -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 Modified: head/sys/conf/files.riscv ============================================================================== --- head/sys/conf/files.riscv Mon May 25 16:06:30 2020 (r361465) +++ head/sys/conf/files.riscv Mon May 25 16:40:48 2020 (r361466) @@ -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 Modified: head/sys/fs/fuse/fuse_vfsops.c ============================================================================== --- head/sys/fs/fuse/fuse_vfsops.c Mon May 25 16:06:30 2020 (r361465) +++ head/sys/fs/fuse/fuse_vfsops.c Mon May 25 16:40:48 2020 (r361466) @@ -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 */ Modified: head/sys/fs/unionfs/union_vfsops.c ============================================================================== --- head/sys/fs/unionfs/union_vfsops.c Mon May 25 16:06:30 2020 (r361465) +++ head/sys/fs/unionfs/union_vfsops.c Mon May 25 16:40:48 2020 (r361466) @@ -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 25 16:06:30 2020 (r361465) +++ head/sys/i386/i386/support.s Mon May 25 16:40:48 2020 (r361466) @@ -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 25 16:06:30 2020 (r361465) +++ head/sys/kern/subr_csan.c Mon May 25 16:40:48 2020 (r361466) @@ -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 25 16:06:30 2020 (r361465) +++ head/sys/mips/mips/support.S Mon May 25 16:40:48 2020 (r361466) @@ -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 25 16:06:30 2020 (r361465) +++ head/sys/sys/systm.h Mon May 25 16:40:48 2020 (r361466) @@ -366,9 +366,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 + 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 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 25 16:40:48 2020 (r361466) @@ -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); +) _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
