From owner-dev-commits-src-all@freebsd.org Tue Jan 5 13:56:47 2021 Return-Path: Delivered-To: dev-commits-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 422814E5BD0; Tue, 5 Jan 2021 13:56:47 +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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D9DZ71Lb2z4VK5; Tue, 5 Jan 2021 13:56:47 +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 1CE9F53C2; Tue, 5 Jan 2021 13:56:47 +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 105DuljM009463; Tue, 5 Jan 2021 13:56:47 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 105Dul1Z009462; Tue, 5 Jan 2021 13:56:47 GMT (envelope-from git) Date: Tue, 5 Jan 2021 13:56:47 GMT Message-Id: <202101051356.105Dul1Z009462@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ed Maste Subject: git: 4696baaaa95c - stable/12 - UFS2: Fix DoS due to corrupted extattrfile MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 4696baaaa95ce9a75a7b1989474382c85981f805 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jan 2021 13:56:47 -0000 The branch stable/12 has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=4696baaaa95ce9a75a7b1989474382c85981f805 commit 4696baaaa95ce9a75a7b1989474382c85981f805 Author: Conrad Meyer AuthorDate: 2020-10-30 19:00:42 +0000 Commit: Ed Maste CommitDate: 2021-01-05 13:56:23 +0000 UFS2: Fix DoS due to corrupted extattrfile Prior versions of FreeBSD (11.x) may have produced a corrupt extattr file. (Specifically, r312416 accidentally fixed this defect by removing a strcpy.) CURRENT FreeBSD supports disk images from those prior versions of FreeBSD. Validate the internal structure as soon as we read it in from disk, to prevent these extattr files from causing invariants violations and DoS. Attempting to access the extattr portion of these files results in EINTEGRITY. At this time, the only way to repair files damaged in this way is to copy the contents to another file and move it over the original. PR: 244089 Reported by: Andrea Venturoli Reviewed by: kib Discussed with: mckusick (earlier draft) (cherry picked from commit e6790841f749b3cc1ac7d338181d9358aae04d0b) --- sys/ufs/ffs/ffs_vnops.c | 31 ++++++++++++++++++++----------- sys/ufs/ufs/extattr.h | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c index d50418fefcad..33f0bb7afedd 100644 --- a/sys/ufs/ffs/ffs_vnops.c +++ b/sys/ufs/ffs/ffs_vnops.c @@ -1141,9 +1141,8 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name, eap = (struct extattr *)ptr; eaend = (struct extattr *)(ptr + length); for (; eap < eaend; eap = EXTATTR_NEXT(eap)) { - /* make sure this entry is complete */ - if (EXTATTR_NEXT(eap) > eaend) - break; + KASSERT(EXTATTR_NEXT(eap) <= eaend, + ("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend)); if (eap->ea_namespace != nspace || eap->ea_namelength != nlen || memcmp(eap->ea_name, name, nlen) != 0) continue; @@ -1157,8 +1156,9 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name, } static int -ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra) +ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td) { + const struct extattr *eap, *eaend, *eapnext; struct inode *ip; struct ufs2_dinode *dp; struct fs *fs; @@ -1172,10 +1172,10 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra) fs = ITOFS(ip); dp = ip->i_din2; easize = dp->di_extsize; - if ((uoff_t)easize + extra > UFS_NXADDR * fs->fs_bsize) + if ((uoff_t)easize > UFS_NXADDR * fs->fs_bsize) return (EFBIG); - eae = malloc(easize + extra, M_TEMP, M_WAITOK); + eae = malloc(easize, M_TEMP, M_WAITOK); liovec.iov_base = eae; liovec.iov_len = easize; @@ -1190,7 +1190,17 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra) error = ffs_extread(vp, &luio, IO_EXT | IO_SYNC); if (error) { free(eae, M_TEMP); - return(error); + return (error); + } + /* Validate disk xattrfile contents. */ + for (eap = (void *)eae, eaend = (void *)(eae + easize); eap < eaend; + eap = eapnext) { + eapnext = EXTATTR_NEXT(eap); + /* Bogusly short entry or bogusly long entry. */ + if (eap->ea_length < sizeof(*eap) || eapnext > eaend) { + free(eae, M_TEMP); + return (EINTEGRITY); + } } *p = eae; return (0); @@ -1241,7 +1251,7 @@ ffs_open_ea(struct vnode *vp, struct ucred *cred, struct thread *td) return (0); } dp = ip->i_din2; - error = ffs_rdextattr(&ip->i_ea_area, vp, td, 0); + error = ffs_rdextattr(&ip->i_ea_area, vp, td); if (error) { ffs_unlock_ea(vp); return (error); @@ -1549,9 +1559,8 @@ vop_listextattr { eap = (struct extattr *)ip->i_ea_area; eaend = (struct extattr *)(ip->i_ea_area + ip->i_ea_len); for (; error == 0 && eap < eaend; eap = EXTATTR_NEXT(eap)) { - /* make sure this entry is complete */ - if (EXTATTR_NEXT(eap) > eaend) - break; + KASSERT(EXTATTR_NEXT(eap) <= eaend, + ("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend)); if (eap->ea_namespace != ap->a_attrnamespace) continue; diff --git a/sys/ufs/ufs/extattr.h b/sys/ufs/ufs/extattr.h index 68b6a424e563..20d3401e68bf 100644 --- a/sys/ufs/ufs/extattr.h +++ b/sys/ufs/ufs/extattr.h @@ -95,7 +95,7 @@ struct extattr { * content referenced by eap. */ #define EXTATTR_NEXT(eap) \ - ((struct extattr *)(((u_char *)(eap)) + (eap)->ea_length)) + ((struct extattr *)(__DECONST(char *, (eap)) + (eap)->ea_length)) #define EXTATTR_CONTENT(eap) \ (void *)(((u_char *)(eap)) + EXTATTR_BASE_LENGTH(eap)) #define EXTATTR_CONTENT_SIZE(eap) \