From nobody Tue Jan 24 22:10:14 2023 X-Original-To: dev-commits-src-branches@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 4P1h3p74HGz3bdgk; Tue, 24 Jan 2023 22:10:14 +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 4P1h3p6Kmjz4JWP; Tue, 24 Jan 2023 22:10:14 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674598214; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=93Du2q+wLSKo/s0LM9n6mMQhoy2V79sHBsv8ZxWVk+0=; b=vtU03Qmbiczrw7YqLyw0RdgeTeCZTCBhDTIL4yNMsMTdkW5p7PPjU61gC5rs3kKpAr0Qmn msCiHc3T3keC7DQ0OlH+uIgjkiIE79zOYuR0WUkjut2xJr3BmD/Wx76V2zCNTeZnhmcOUK aZ9FAKp9NJS6u47XHAHHIRY9BkHmfmf6RPgKIkp3Wl9VkOe1Xv1KYxiR9EfKaT6RPhLqif UaJIo62bVVk8aZd4q/RMcOjJjmrO6xTjpECqErL65CH03vbbNn9OZOdihF5ATxd0GMIzLM /XJZFkF+sahBb0WUaRjiOFO8CHdkT6x1lTFieEgNvlpTw7xHmCb7HYb4FmAomg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674598214; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=93Du2q+wLSKo/s0LM9n6mMQhoy2V79sHBsv8ZxWVk+0=; b=xqr7+zZppP4bA0sreZNjbKeinhhJ2dMYOyIDg2Auz/3lhKxIWm6TIt9Wxn8DegG2PbrWsH LWb+e40Fj1isWyIwQoyHtLjRVcGjEO5CaG7HoGCbXshblVsahym8No6K2hHgwSEuKRaYhf xMXSfoPhKlyhFAX/TTB/1W4V3oqaALm6qOCr9qyuMee/7eJQwF1b9XyZ6GzrSlvZc5x6ll J3PABvIWGAclcYiuaWN3VHM7ZZJzaxUfrpl9PH+Y3aIvMcjoMqpvXQd8xW6ZgiuBgntTIj 0pFDXuWR8ToAa7e+yEKJrd1ZAdLKckujyVaZxeAIqNRn8JeCGJag0GJT3UnpKg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1674598214; a=rsa-sha256; cv=none; b=Q8Zh98znYZXe0vDKXpsUEu3SkzpUHKkWufNNBLrFuMFVDaw/NfKJl97uALND3EnIvkjbdd UHsmaZnuh+v0oo3bxIUb/ZzsHC4gJgiiGy6B4IirKCSfqc4sJTJ6sVnI3PjAK/otJR9x5b tG29W3lKj6q/xw/UiAzkc4AD6whOyxXnV6U1TlYfny7kMq/sRTJ/lcLX9ZvgK+Wnaw9CAe ZxRf0MAsYmzCsTkrh+HoRGWrT8smqRYYiGyxYs2yzXwxNpSImiuVUY9O4+VeyuK0XweoDk oNSc1qyc3fh4QnL4nwUZD3W21AVMaCEgmKHxCKZ7+CchzYxb5MswXkvs/3qFYQ== 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 4P1h3p5QW5zQWb; Tue, 24 Jan 2023 22:10:14 +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 30OMAEt1078300; Tue, 24 Jan 2023 22:10:14 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 30OMAEb9078294; Tue, 24 Jan 2023 22:10:14 GMT (envelope-from git) Date: Tue, 24 Jan 2023 22:10:14 GMT Message-Id: <202301242210.30OMAEb9078294@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Warner Losh Subject: git: 3622fe162012 - stable/13 - ufs: Rework shortlink handling to avoid subobject overflows List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 3622fe16201215cd511112be0fcc48ed1576daee Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=3622fe16201215cd511112be0fcc48ed1576daee commit 3622fe16201215cd511112be0fcc48ed1576daee Author: Jessica Clarke AuthorDate: 2022-01-02 20:55:36 +0000 Commit: Warner Losh CommitDate: 2023-01-24 21:49:19 +0000 ufs: Rework shortlink handling to avoid subobject overflows Shortlinks occupy the space of both di_db and di_ib when used. However, everywhere that wants to read or write a shortlink takes a pointer do di_db and promptly runs off the end of it into di_ib. This is fine on most architectures, if a little dodgy. However, on CHERI, the compiler can optionally restrict the bounds on pointers to subobjects to just that subobject, in order to mitigate intra-object buffer overflows, and this is enabled in CheriBSD's pure-capability kernels. Instead, clean this up by inserting a union such that a new di_shortlink can be added with the right size and element type, avoiding the need to cast and allowing the use of the DIP macro to access the field. This also mirrors how the ext2fs code implements extents support, with the exact same structure other than having a uint32_t i_data[] instead of a char di_shortlink[]. Reviewed by: mckusick, jhb Differential Revision: https://reviews.freebsd.org/D33650 (cherry picked from commit 5b13fa7987c13aa7b5a67cc6b465475912de2d14) --- sbin/dump/traverse.c | 8 ++------ sbin/fsdb/fsdbutil.c | 7 ++----- stand/libsa/ufs.c | 7 ++----- sys/ufs/ffs/ffs_inode.c | 2 +- sys/ufs/ufs/dinode.h | 24 ++++++++++++++++++++---- sys/ufs/ufs/inode.h | 2 -- sys/ufs/ufs/ufs_vnops.c | 4 ++-- tools/diag/prtblknos/prtblknos.c | 4 ++-- usr.sbin/makefs/ffs.c | 4 ++-- usr.sbin/makefs/ffs/ufs_inode.h | 4 ++-- 10 files changed, 35 insertions(+), 31 deletions(-) diff --git a/sbin/dump/traverse.c b/sbin/dump/traverse.c index 3630d2240f58..08e902667759 100644 --- a/sbin/dump/traverse.c +++ b/sbin/dump/traverse.c @@ -525,12 +525,8 @@ dumpino(union dinode *dp, ino_t ino) spcl.c_count = 1; added = appendextdata(dp); writeheader(ino); - if (sblock->fs_magic == FS_UFS1_MAGIC) - memmove(buf, (caddr_t)dp->dp1.di_db, - (u_long)DIP(dp, di_size)); - else - memmove(buf, (caddr_t)dp->dp2.di_db, - (u_long)DIP(dp, di_size)); + memmove(buf, DIP(dp, di_shortlink), + (u_long)DIP(dp, di_size)); buf[DIP(dp, di_size)] = '\0'; writerec(buf, 0); writeextdata(dp, ino, added); diff --git a/sbin/fsdb/fsdbutil.c b/sbin/fsdb/fsdbutil.c index 2c18d9910c6f..c8a3a8a525e3 100644 --- a/sbin/fsdb/fsdbutil.c +++ b/sbin/fsdb/fsdbutil.c @@ -136,11 +136,8 @@ printstat(const char *cp, ino_t inum, union dinode *dp) if (DIP(dp, di_size) > 0 && DIP(dp, di_size) < sblock.fs_maxsymlinklen && DIP(dp, di_blocks) == 0) { - if (sblock.fs_magic == FS_UFS1_MAGIC) - p = (caddr_t)dp->dp1.di_db; - else - p = (caddr_t)dp->dp2.di_db; - printf(" to `%.*s'\n", (int) DIP(dp, di_size), p); + printf(" to `%.*s'\n", (int) DIP(dp, di_size), + DIP(dp, di_shortlink)); } else { putchar('\n'); } diff --git a/stand/libsa/ufs.c b/stand/libsa/ufs.c index a4015dea74c2..fa3820617860 100644 --- a/stand/libsa/ufs.c +++ b/stand/libsa/ufs.c @@ -643,11 +643,8 @@ ufs_open(const char *upath, struct open_file *f) bcopy(cp, &namebuf[link_len], len + 1); if (link_len < fs->fs_maxsymlinklen) { - if (fp->f_fs->fs_magic == FS_UFS1_MAGIC) - cp = (caddr_t)(fp->f_di.di1.di_db); - else - cp = (caddr_t)(fp->f_di.di2.di_db); - bcopy(cp, namebuf, (unsigned) link_len); + bcopy(DIP(fp, di_shortlink), namebuf, + (unsigned) link_len); } else { /* * Read file for symbolic link diff --git a/sys/ufs/ffs/ffs_inode.c b/sys/ufs/ffs/ffs_inode.c index 1aef2bb57930..d9d4f3c4a155 100644 --- a/sys/ufs/ffs/ffs_inode.c +++ b/sys/ufs/ffs/ffs_inode.c @@ -339,7 +339,7 @@ ffs_truncate(struct vnode *vp, if (length != 0) panic("ffs_truncate: partial truncate of symlink"); #endif - bzero(SHORTLINK(ip), (u_int)ip->i_size); + bzero(DIP(ip, i_shortlink), (u_int)ip->i_size); ip->i_size = 0; DIP_SET(ip, i_size, 0); UFS_INODE_SET_FLAG(ip, IN_SIZEMOD | IN_CHANGE | IN_UPDATE); diff --git a/sys/ufs/ufs/dinode.h b/sys/ufs/ufs/dinode.h index 1f0f25c4d5ec..840a4cc7d40f 100644 --- a/sys/ufs/ufs/dinode.h +++ b/sys/ufs/ufs/dinode.h @@ -145,8 +145,16 @@ struct ufs2_dinode { u_int32_t di_flags; /* 88: Status flags (chflags). */ u_int32_t di_extsize; /* 92: External attributes size. */ ufs2_daddr_t di_extb[UFS_NXADDR];/* 96: External attributes block. */ - ufs2_daddr_t di_db[UFS_NDADDR]; /* 112: Direct disk blocks. */ - ufs2_daddr_t di_ib[UFS_NIADDR]; /* 208: Indirect disk blocks. */ + union { + struct { + ufs2_daddr_t di_db /* 112: Direct disk blocks. */ + [UFS_NDADDR]; + ufs2_daddr_t di_ib /* 208: Indirect disk blocks. */ + [UFS_NIADDR]; + }; + char di_shortlink /* 112: Embedded symbolic link. */ + [(UFS_NDADDR + UFS_NIADDR) * sizeof(ufs2_daddr_t)]; + }; u_int64_t di_modrev; /* 232: i_modrev for NFSv4 */ uint32_t di_freelink; /* 240: SUJ: Next unlinked inode. */ uint32_t di_ckhash; /* 244: if CK_INODE, its check-hash */ @@ -179,8 +187,16 @@ struct ufs1_dinode { int32_t di_mtimensec; /* 28: Last modified time. */ int32_t di_ctime; /* 32: Last inode change time. */ int32_t di_ctimensec; /* 36: Last inode change time. */ - ufs1_daddr_t di_db[UFS_NDADDR]; /* 40: Direct disk blocks. */ - ufs1_daddr_t di_ib[UFS_NIADDR]; /* 88: Indirect disk blocks. */ + union { + struct { + ufs1_daddr_t di_db /* 40: Direct disk blocks. */ + [UFS_NDADDR]; + ufs1_daddr_t di_ib /* 88: Indirect disk blocks. */ + [UFS_NIADDR]; + }; + char di_shortlink /* 40: Embedded symbolic link. */ + [(UFS_NDADDR + UFS_NIADDR) * sizeof(ufs1_daddr_t)]; + }; u_int32_t di_flags; /* 100: Status flags (chflags). */ u_int32_t di_blocks; /* 104: Blocks actually held. */ u_int32_t di_gen; /* 108: Generation number. */ diff --git a/sys/ufs/ufs/inode.h b/sys/ufs/ufs/inode.h index 3e555dc52a32..8cdd40894608 100644 --- a/sys/ufs/ufs/inode.h +++ b/sys/ufs/ufs/inode.h @@ -242,8 +242,6 @@ I_IS_UFS2(const struct inode *ip) (ip)->i_din2->d##field = (val); \ } while (0) -#define SHORTLINK(ip) (I_IS_UFS1(ip) ? \ - (caddr_t)(ip)->i_din1->di_db : (caddr_t)(ip)->i_din2->di_db) #define IS_SNAPSHOT(ip) ((ip)->i_flags & SF_SNAPSHOT) #define IS_UFS(vp) ((vp)->v_data != NULL) diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 93ac04941db7..57560efffbbf 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -2387,7 +2387,7 @@ ufs_symlink( len = strlen(ap->a_target); if (len < VFSTOUFS(vp->v_mount)->um_maxsymlinklen) { ip = VTOI(vp); - bcopy(ap->a_target, SHORTLINK(ip), len); + bcopy(ap->a_target, DIP(ip, i_shortlink), len); ip->i_size = len; DIP_SET(ip, i_size, len); UFS_INODE_SET_FLAG(ip, IN_SIZEMOD | IN_CHANGE | IN_UPDATE); @@ -2557,7 +2557,7 @@ ufs_readlink( isize = ip->i_size; if (isize < VFSTOUFS(vp->v_mount)->um_maxsymlinklen) - return (uiomove(SHORTLINK(ip), isize, ap->a_uio)); + return (uiomove(DIP(ip, i_shortlink), isize, ap->a_uio)); return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred)); } diff --git a/tools/diag/prtblknos/prtblknos.c b/tools/diag/prtblknos/prtblknos.c index 771d79e43ec9..ae53471156a6 100644 --- a/tools/diag/prtblknos/prtblknos.c +++ b/tools/diag/prtblknos/prtblknos.c @@ -99,8 +99,8 @@ prtblknos(fs, dp) if (size < fs->fs_maxsymlinklen) { printf("symbolic link referencing %s\n", (fs->fs_magic == FS_UFS1_MAGIC) ? - (char *)dp->dp1.di_db : - (char *)dp->dp2.di_db); + dp->dp1.di_shortlink : + dp->dp2.di_shortlink); return; } printf("symbolic link\n"); diff --git a/usr.sbin/makefs/ffs.c b/usr.sbin/makefs/ffs.c index d91ed156c9e0..1911f75fb4fd 100644 --- a/usr.sbin/makefs/ffs.c +++ b/usr.sbin/makefs/ffs.c @@ -714,7 +714,7 @@ ffs_build_dinode1(struct ufs1_dinode *dinp, dirbuf_t *dbufp, fsnode *cur, } else if (S_ISLNK(cur->type)) { /* symlink */ slen = strlen(cur->symlink); if (slen < UFS1_MAXSYMLINKLEN) { /* short link */ - memcpy(dinp->di_db, cur->symlink, slen); + memcpy(dinp->di_shortlink, cur->symlink, slen); } else membuf = cur->symlink; dinp->di_size = slen; @@ -773,7 +773,7 @@ ffs_build_dinode2(struct ufs2_dinode *dinp, dirbuf_t *dbufp, fsnode *cur, } else if (S_ISLNK(cur->type)) { /* symlink */ slen = strlen(cur->symlink); if (slen < UFS2_MAXSYMLINKLEN) { /* short link */ - memcpy(dinp->di_db, cur->symlink, slen); + memcpy(dinp->di_shortlink, cur->symlink, slen); } else membuf = cur->symlink; dinp->di_size = slen; diff --git a/usr.sbin/makefs/ffs/ufs_inode.h b/usr.sbin/makefs/ffs/ufs_inode.h index 07c1ff63b865..e7baff23cbfc 100644 --- a/usr.sbin/makefs/ffs/ufs_inode.h +++ b/usr.sbin/makefs/ffs/ufs_inode.h @@ -68,7 +68,7 @@ struct inode { #define i_ffs1_mtimensec i_din.ffs1_din.di_mtimensec #define i_ffs1_nlink i_din.ffs1_din.di_nlink #define i_ffs1_rdev i_din.ffs1_din.di_rdev -#define i_ffs1_shortlink i_din.ffs1_din.db +#define i_ffs1_shortlink i_din.ffs1_din.di_shortlink #define i_ffs1_size i_din.ffs1_din.di_size #define i_ffs1_uid i_din.ffs1_din.di_uid @@ -89,7 +89,7 @@ struct inode { #define i_ffs2_mtimensec i_din.ffs2_din.di_mtimensec #define i_ffs2_nlink i_din.ffs2_din.di_nlink #define i_ffs2_rdev i_din.ffs2_din.di_rdev -#define i_ffs2_shortlink i_din.ffs2_din.db +#define i_ffs2_shortlink i_din.ffs2_din.di_shortlink #define i_ffs2_size i_din.ffs2_din.di_size #define i_ffs2_uid i_din.ffs2_din.di_uid