git: 5b13fa7987c1 - main - ufs: Rework shortlink handling to avoid subobject overflows
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 02 Jan 2022 20:56:16 UTC
The branch main has been updated by jrtc27:
URL: https://cgit.FreeBSD.org/src/commit/?id=5b13fa7987c13aa7b5a67cc6b465475912de2d14
commit 5b13fa7987c13aa7b5a67cc6b465475912de2d14
Author: Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2022-01-02 20:55:36 +0000
Commit: Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2022-01-02 20:55:36 +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
---
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 31212bf3473a..12703a3e18aa 100644
--- a/stand/libsa/ufs.c
+++ b/stand/libsa/ufs.c
@@ -638,11 +638,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 4b31b4febcbd..d8b86df27121 100644
--- a/sys/ufs/ffs/ffs_inode.c
+++ b/sys/ufs/ffs/ffs_inode.c
@@ -342,7 +342,7 @@ ffs_truncate(vp, length, flags, cred)
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 a82b4658c0af..7ea88d15b1e3 100644
--- a/sys/ufs/ufs/inode.h
+++ b/sys/ufs/ufs/inode.h
@@ -246,8 +246,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)
/*
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index 89454d0effff..1d0525bcbe0c 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -2311,7 +2311,7 @@ ufs_symlink(ap)
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);
@@ -2481,7 +2481,7 @@ ufs_readlink(ap)
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 4496cabcd83e..9cfbcbc30d75 100644
--- a/usr.sbin/makefs/ffs.c
+++ b/usr.sbin/makefs/ffs.c
@@ -704,7 +704,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;
@@ -763,7 +763,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 e4caaafa5b1a..2b30b801b36e 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