From nobody Thu Oct 07 21:18:09 2021 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 8B97212BE7CD; Thu, 7 Oct 2021 21:18:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HQPLT2z0Kz554w; Thu, 7 Oct 2021 21:18:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2F8101A9D1; Thu, 7 Oct 2021 21:18:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 197LI9xC090142; Thu, 7 Oct 2021 21:18:09 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 197LI9Mq090141; Thu, 7 Oct 2021 21:18:09 GMT (envelope-from git) Date: Thu, 7 Oct 2021 21:18:09 GMT Message-Id: <202110072118.197LI9Mq090141@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mitchell Horne Subject: git: 8babb5582eed - main - riscv: fix VM_MAXUSER_ADDRESS checks in asm routines List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mhorne X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8babb5582eed2250309084d76898798409a2aae0 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mhorne: URL: https://cgit.FreeBSD.org/src/commit/?id=8babb5582eed2250309084d76898798409a2aae0 commit 8babb5582eed2250309084d76898798409a2aae0 Author: Mitchell Horne AuthorDate: 2021-10-07 21:12:30 +0000 Commit: Mitchell Horne CommitDate: 2021-10-07 21:12:30 +0000 riscv: fix VM_MAXUSER_ADDRESS checks in asm routines There are two issues with the checks against VM_MAXUSER_ADDRESS. First, the comparison should consider the values as unsigned, otherwise addresses with the high bit set will fail to branch. Second, the value of VM_MAXUSER_ADDRESS is, by convention, one larger than the maximum mappable user address and invalid itself. Thus, use the bgeu instruction for these comparisons. Add a regression test case for copyin(9). PR: 257193 Reported by: Robert Morris Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D31209 --- sys/riscv/riscv/copyinout.S | 6 +++--- sys/riscv/riscv/support.S | 20 ++++++++++---------- tests/sys/kern/kern_copyin.c | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/sys/riscv/riscv/copyinout.S b/sys/riscv/riscv/copyinout.S index 1f2a44121ecd..5a171f5a5e17 100644 --- a/sys/riscv/riscv/copyinout.S +++ b/sys/riscv/riscv/copyinout.S @@ -118,7 +118,7 @@ ENTRY(copyout) beqz a2, copyout_end /* If len == 0 then skip loop */ add a3, a1, a2 li a4, VM_MAXUSER_ADDRESS - bgt a3, a4, copyio_fault_nopcb + bgeu a3, a4, copyio_fault_nopcb copycommon @@ -136,7 +136,7 @@ ENTRY(copyin) beqz a2, copyin_end /* If len == 0 then skip loop */ add a3, a0, a2 li a4, VM_MAXUSER_ADDRESS - bgt a3, a4, copyio_fault_nopcb + bgeu a3, a4, copyio_fault_nopcb copycommon @@ -159,7 +159,7 @@ ENTRY(copyinstr) ENTER_USER_ACCESS(a7) li a7, VM_MAXUSER_ADDRESS -1: bgt a0, a7, copyio_fault +1: bgeu a0, a7, copyio_fault lb a4, 0(a0) /* Load from uaddr */ addi a0, a0, 1 sb a4, 0(a1) /* Store in kaddr */ diff --git a/sys/riscv/riscv/support.S b/sys/riscv/riscv/support.S index 3f0ec08ac768..7fcd6af283b7 100644 --- a/sys/riscv/riscv/support.S +++ b/sys/riscv/riscv/support.S @@ -56,7 +56,7 @@ END(fsu_fault) */ ENTRY(casueword32) li a4, (VM_MAXUSER_ADDRESS-3) - bgt a0, a4, fsu_fault_nopcb + bgeu a0, a4, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a4) /* And set it */ ENTER_USER_ACCESS(a4) @@ -77,7 +77,7 @@ END(casueword32) */ ENTRY(casueword) li a4, (VM_MAXUSER_ADDRESS-7) - bgt a0, a4, fsu_fault_nopcb + bgeu a0, a4, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a4) /* And set it */ ENTER_USER_ACCESS(a4) @@ -98,7 +98,7 @@ END(casueword) */ ENTRY(fubyte) li a1, VM_MAXUSER_ADDRESS - bgt a0, a1, fsu_fault_nopcb + bgeu a0, a1, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a1) /* And set it */ ENTER_USER_ACCESS(a1) @@ -113,7 +113,7 @@ END(fubyte) */ ENTRY(fuword16) li a1, (VM_MAXUSER_ADDRESS-1) - bgt a0, a1, fsu_fault_nopcb + bgeu a0, a1, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a1) /* And set it */ ENTER_USER_ACCESS(a1) @@ -128,7 +128,7 @@ END(fuword16) */ ENTRY(fueword32) li a2, (VM_MAXUSER_ADDRESS-3) - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) @@ -147,7 +147,7 @@ END(fueword32) ENTRY(fueword) EENTRY(fueword64) li a2, (VM_MAXUSER_ADDRESS-7) - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) @@ -165,7 +165,7 @@ END(fueword) */ ENTRY(subyte) li a2, VM_MAXUSER_ADDRESS - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) @@ -181,7 +181,7 @@ END(subyte) */ ENTRY(suword16) li a2, (VM_MAXUSER_ADDRESS-1) - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) @@ -197,7 +197,7 @@ END(suword16) */ ENTRY(suword32) li a2, (VM_MAXUSER_ADDRESS-3) - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) @@ -214,7 +214,7 @@ END(suword32) ENTRY(suword) EENTRY(suword64) li a2, (VM_MAXUSER_ADDRESS-7) - bgt a0, a2, fsu_fault_nopcb + bgeu a0, a2, fsu_fault_nopcb la a6, fsu_fault /* Load the fault handler */ SET_FAULT_HANDLER(a6, a2) /* And set it */ ENTER_USER_ACCESS(a2) diff --git a/tests/sys/kern/kern_copyin.c b/tests/sys/kern/kern_copyin.c index b77360e928fd..eb1fea315b5a 100644 --- a/tests/sys/kern/kern_copyin.c +++ b/tests/sys/kern/kern_copyin.c @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -54,6 +55,21 @@ copyin_checker(uintptr_t uaddr, size_t len) return (ret == -1 ? errno : 0); } +#if __SIZEOF_POINTER__ == 8 +/* + * A slightly more direct path to calling copyin(), but without the ability + * to specify a length. + */ +static int +copyin_checker2(uintptr_t uaddr) +{ + int ret; + + ret = fcntl(scratch_file, F_GETLK, (const void *)uaddr); + return (ret == -1 ? errno : 0); +} +#endif + #ifdef __amd64__ static uintptr_t get_maxuser_address(void) @@ -83,6 +99,10 @@ get_maxuser_address(void) #endif #define FMAX ULONG_MAX +#if __SIZEOF_POINTER__ == 8 +/* PR 257193 */ +#define ADDR_SIGNED 0x800000c000000000 +#endif ATF_TC_WITHOUT_HEAD(kern_copyin); ATF_TC_BODY(kern_copyin, tc) @@ -122,6 +142,10 @@ ATF_TC_BODY(kern_copyin, tc) ATF_CHECK(copyin_checker(FMAX - 10, 9) == EFAULT); ATF_CHECK(copyin_checker(FMAX - 10, 10) == EFAULT); ATF_CHECK(copyin_checker(FMAX - 10, 11) == EFAULT); +#if __SIZEOF_POINTER__ == 8 + ATF_CHECK(copyin_checker(ADDR_SIGNED, 1) == EFAULT); + ATF_CHECK(copyin_checker2(ADDR_SIGNED) == EFAULT); +#endif } ATF_TP_ADD_TCS(tp)