From owner-svn-src-head@freebsd.org Fri Oct 6 14:29:54 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 54B39E389EC; Fri, 6 Oct 2017 14:29:54 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 2F09E1D98; Fri, 6 Oct 2017 14:29:54 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v96ETrp2099071; Fri, 6 Oct 2017 14:29:53 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v96ETrcr099070; Fri, 6 Oct 2017 14:29:53 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201710061429.v96ETrcr099070@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Fri, 6 Oct 2017 14:29:53 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r324366 - head/sys/i386/i386 X-SVN-Group: head X-SVN-Commit-Author: kib X-SVN-Commit-Paths: head/sys/i386/i386 X-SVN-Commit-Revision: 324366 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Oct 2017 14:29:54 -0000 Author: kib Date: Fri Oct 6 14:29:53 2017 New Revision: 324366 URL: https://svnweb.freebsd.org/changeset/base/324366 Log: Improve i386_get_ldt(). Provide consistent snapshot of the requested descriptors by preventing other threads from modifying LDT while we fetch the data, lock dt_lock around the read. Copy the data into intermediate buffer, which is copied out after the lock is dropped. Comparing with the amd64 version, the read is done byte by byte, since there is no atomic 64bit read (cmpxchg8b method is too heavy comparing with the avoided issues). Improve overflow checking for the descriptors range calculations and remove unneeded casts. Use unsigned types for sizes. Allow zero num argument to i386_get_ldt() and i386_set_ldt(). This case is handled naturally by the code flow. Reviewed by: bde Sponsored by: The FreeBSD Foundation MFC after: 1 week Modified: head/sys/i386/i386/sys_machdep.c Modified: head/sys/i386/i386/sys_machdep.c ============================================================================== --- head/sys/i386/i386/sys_machdep.c Fri Oct 6 13:48:38 2017 (r324365) +++ head/sys/i386/i386/sys_machdep.c Fri Oct 6 14:29:53 2017 (r324366) @@ -154,8 +154,6 @@ sysarch(struct thread *td, struct sysarch_args *uap) if ((error = copyin(uap->parms, &kargs.largs, sizeof(struct i386_ldt_args))) != 0) return (error); - if (kargs.largs.num > MAX_LD || kargs.largs.num <= 0) - return (EINVAL); break; case I386_GET_XFPUSTATE: if ((error = copyin(uap->parms, &kargs.xfpu, @@ -172,6 +170,8 @@ sysarch(struct thread *td, struct sysarch_args *uap) break; case I386_SET_LDT: if (kargs.largs.descs != NULL) { + if (kargs.largs.num > MAX_LD) + return (EINVAL); lp = malloc(kargs.largs.num * sizeof(union descriptor), M_TEMP, M_WAITOK); error = copyin(kargs.largs.descs, lp, @@ -503,38 +503,37 @@ user_ldt_deref(struct proc_ldt *pldt) int i386_get_ldt(struct thread *td, struct i386_ldt_args *uap) { - int error = 0; struct proc_ldt *pldt; - int nldt, num; - union descriptor *lp; + char *data; + u_int nldt, num; + int error; #ifdef DEBUG printf("i386_get_ldt: start=%u num=%u descs=%p\n", uap->start, uap->num, (void *)uap->descs); #endif + if (uap->start >= MAX_LD) + return (EINVAL); + num = min(uap->num, MAX_LD - uap->start); + data = malloc(uap->num * sizeof(union descriptor), M_TEMP, M_WAITOK); mtx_lock_spin(&dt_lock); - if ((pldt = td->td_proc->p_md.md_ldt) != NULL) { - nldt = pldt->ldt_len; - lp = &((union descriptor *)(pldt->ldt_base))[uap->start]; + pldt = td->td_proc->p_md.md_ldt; + nldt = pldt != NULL ? pldt->ldt_len : nitems(ldt); + num = min(num, nldt); + if (uap->start > nldt || uap->start + num > nldt) { mtx_unlock_spin(&dt_lock); - num = min(uap->num, nldt); - } else { - mtx_unlock_spin(&dt_lock); - nldt = sizeof(ldt)/sizeof(ldt[0]); - num = min(uap->num, nldt); - lp = &ldt[uap->start]; + return (EINVAL); } + bcopy(pldt != NULL ? + &((union descriptor *)(pldt->ldt_base))[uap->start] : + &ldt[uap->start], data, num * sizeof(union descriptor)); + mtx_unlock_spin(&dt_lock); - if ((uap->start > (unsigned int)nldt) || - ((unsigned int)num > (unsigned int)nldt) || - ((unsigned int)(uap->start + num) > (unsigned int)nldt)) - return(EINVAL); - - error = copyout(lp, uap->descs, num * sizeof(union descriptor)); + error = copyout(data, uap->descs, num * sizeof(union descriptor)); if (error == 0) td->td_retval[0] = num; - + free(data, M_TEMP); return (error); } @@ -542,11 +541,11 @@ int i386_set_ldt(struct thread *td, struct i386_ldt_args *uap, union descriptor *descs) { - int error, i; - int largest_ld; struct mdproc *mdp; struct proc_ldt *pldt; union descriptor *dp; + u_int largest_ld, i; + int error; #ifdef DEBUG printf("i386_set_ldt: start=%u num=%u descs=%p\n", @@ -565,8 +564,6 @@ i386_set_ldt(struct thread *td, struct i386_ldt_args * uap->start = NLDT; uap->num = MAX_LD - NLDT; } - if (uap->num == 0) - return (EINVAL); mtx_lock_spin(&dt_lock); if ((pldt = mdp->md_ldt) == NULL || uap->start >= pldt->ldt_len) {