From owner-svn-src-all@freebsd.org Sat Jan 11 15:06:07 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B8AAA1ED1DF; Sat, 11 Jan 2020 15:06:07 +0000 (UTC) (envelope-from kevans@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47w38H4Sf9z49X2; Sat, 11 Jan 2020 15:06:07 +0000 (UTC) (envelope-from kevans@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 935DD4B72; Sat, 11 Jan 2020 15:06:07 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 00BF67EF004763; Sat, 11 Jan 2020 15:06:07 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 00BF67tN004761; Sat, 11 Jan 2020 15:06:07 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <202001111506.00BF67tN004761@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Sat, 11 Jan 2020 15:06:07 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r356634 - in stable: 11/sys/compat/linux 11/sys/sys 11/sys/vm 12/sys/compat/linux 12/sys/sys 12/sys/vm X-SVN-Group: stable-11 X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: in stable: 11/sys/compat/linux 11/sys/sys 11/sys/vm 12/sys/compat/linux 12/sys/sys 12/sys/vm X-SVN-Commit-Revision: 356634 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 Jan 2020 15:06:07 -0000 Author: kevans Date: Sat Jan 11 15:06:06 2020 New Revision: 356634 URL: https://svnweb.freebsd.org/changeset/base/356634 Log: MFC r356359-r356360: kern_mmap: add fpcheck invariant for linux_mmap r356359: kern_mmap: add a variant that allows caller to inspect fp Linux mmap rejects mmap() on a write-only file with EACCES. linux_mmap_common currently does a fun dance to grab the fp associated with the passed in fd, validates it, then drops the reference and calls into kern_mmap(). Doing so is perhaps both fragile and premature; there's still plenty of chance for the request to get rejected with a more appropriate error, and it's prone to a race where the file we ultimately mmap has changed after it drops its referenced. This change alleviates the need to do this by providing a kern_mmap variant that allows the caller to inspect the fp just before calling into the fileop layer. The callback takes flags, prot, and maxprot as one could imagine scenarios where any of these, in conjunction with the file itself, may influence a caller's decision. The file type check in the linux compat layer has been removed; EINVAL is seemingly not an appropriate response to the file not being a vnode or device. The fileop layer will reject the operation with ENODEV if it's not supported, which more closely matches the common linux description of mmap(2) return values. If we discover that we're allowing an mmap() on a file type that Linux normally wouldn't, we should restrict those explicitly. r356360: kern_mmap: restore character deleted in transit Modified: stable/11/sys/compat/linux/linux_mmap.c stable/11/sys/sys/syscallsubr.h stable/11/sys/vm/vm_mmap.c Directory Properties: stable/11/ (props changed) Changes in other areas also in this revision: Modified: stable/12/sys/compat/linux/linux_mmap.c stable/12/sys/sys/syscallsubr.h stable/12/sys/vm/vm_mmap.c Directory Properties: stable/12/ (props changed) Modified: stable/11/sys/compat/linux/linux_mmap.c ============================================================================== --- stable/11/sys/compat/linux/linux_mmap.c Sat Jan 11 12:02:16 2020 (r356633) +++ stable/11/sys/compat/linux/linux_mmap.c Sat Jan 11 15:06:06 2020 (r356634) @@ -62,7 +62,17 @@ __FBSDID("$FreeBSD$"); static void linux_fixup_prot(struct thread *td, int *prot); #endif +static int +linux_mmap_check_fp(struct file *fp, int flags, int prot, int maxprot) +{ + /* Linux mmap() just fails for O_WRONLY files */ + if ((fp->f_flag & FREAD) == 0) + return (EACCES); + + return (0); +} + int linux_mmap_common(struct thread *td, uintptr_t addr, size_t len, int prot, int flags, int fd, off_t pos) @@ -118,31 +128,6 @@ linux_mmap_common(struct thread *td, uintptr_t addr, s /* Linux does not check file descriptor when MAP_ANONYMOUS is set. */ fd = (bsd_flags & MAP_ANON) ? -1 : fd; - if (fd != -1) { - /* - * Linux follows Solaris mmap(2) description: - * The file descriptor fildes is opened with - * read permission, regardless of the - * protection options specified. - */ - - error = fget(td, fd, cap_rights_init(&rights, CAP_MMAP), &fp); - if (error != 0) - return (error); - if (fp->f_type != DTYPE_VNODE && fp->f_type != DTYPE_DEV) { - fdrop(fp, td); - return (EINVAL); - } - - /* Linux mmap() just fails for O_WRONLY files */ - if (!(fp->f_flag & FREAD)) { - fdrop(fp, td); - return (EACCES); - } - - fdrop(fp, td); - } - if (flags & LINUX_MAP_GROWSDOWN) { /* * The Linux MAP_GROWSDOWN option does not limit auto @@ -212,13 +197,15 @@ linux_mmap_common(struct thread *td, uintptr_t addr, s */ if (addr != 0 && (bsd_flags & MAP_FIXED) == 0 && (bsd_flags & MAP_EXCL) == 0) { - error = kern_mmap(td, addr, len, prot, - bsd_flags | MAP_FIXED | MAP_EXCL, fd, pos); + error = kern_mmap_fpcheck(td, addr, len, prot, + bsd_flags | MAP_FIXED | MAP_EXCL, fd, pos, + linux_mmap_check_fp); if (error == 0) goto out; } - error = kern_mmap(td, addr, len, prot, bsd_flags, fd, pos); + error = kern_mmap_fpcheck(td, addr, len, prot, bsd_flags, fd, pos, + linux_mmap_check_fp); out: LINUX_CTR2(mmap2, "return: %d (%p)", error, td->td_retval[0]); Modified: stable/11/sys/sys/syscallsubr.h ============================================================================== --- stable/11/sys/sys/syscallsubr.h Sat Jan 11 12:02:16 2020 (r356633) +++ stable/11/sys/sys/syscallsubr.h Sat Jan 11 15:06:06 2020 (r356634) @@ -60,6 +60,8 @@ struct thr_param; struct sched_param; struct __wrusage; +typedef int (*mmap_check_fp_fn)(struct file *, int, int, int); + int kern___getcwd(struct thread *td, char *buf, enum uio_seg bufseg, u_int buflen, u_int path_max); int kern_accept(struct thread *td, int s, struct sockaddr **name, @@ -165,6 +167,9 @@ int kern_mlock(struct proc *proc, struct ucred *cred, size_t len); int kern_mmap(struct thread *td, uintptr_t addr, size_t size, int prot, int flags, int fd, off_t pos); +int kern_mmap_fpcheck(struct thread *td, uintptr_t addr, size_t len, + int prot, int flags, int fd, off_t pos, + mmap_check_fp_fn check_fp_fn); int kern_mprotect(struct thread *td, uintptr_t addr, size_t size, int prot); int kern_msgctl(struct thread *, int, int, struct msqid_ds *); int kern_msgrcv(struct thread *, int, void *, size_t, long, int, long *); Modified: stable/11/sys/vm/vm_mmap.c ============================================================================== --- stable/11/sys/vm/vm_mmap.c Sat Jan 11 12:02:16 2020 (r356633) +++ stable/11/sys/vm/vm_mmap.c Sat Jan 11 15:06:06 2020 (r356634) @@ -184,6 +184,18 @@ int kern_mmap(struct thread *td, uintptr_t addr0, size_t size, int prot, int flags, int fd, off_t pos) { + + return (kern_mmap_fpcheck(td, addr0, size, prot, flags, fd, pos, NULL)); +} + +/* + * When mmap'ing a file, check_fp_fn may be used for the caller to do any + * last-minute validation based on the referenced file in a non-racy way. + */ +int +kern_mmap_fpcheck(struct thread *td, uintptr_t addr0, size_t size, int prot, + int flags, int fd, off_t pos, mmap_check_fp_fn check_fp_fn) +{ struct vmspace *vms; struct file *fp; vm_offset_t addr; @@ -356,7 +368,11 @@ kern_mmap(struct thread *td, uintptr_t addr0, size_t s error = EINVAL; goto done; } - + if (check_fp_fn != NULL) { + error = check_fp_fn(fp, prot, cap_maxprot, flags); + if (error != 0) + goto done; + } /* This relies on VM_PROT_* matching PROT_*. */ error = fo_mmap(fp, &vms->vm_map, &addr, size, prot, cap_maxprot, flags, pos, td);