From eb6bf40db8c2c6a5621845360152dd04b815759c Mon Sep 17 00:00:00 2001 From: gordon Date: Wed, 3 Jul 2019 00:02:16 +0000 Subject: [PATCH] Fix kernel stack disclosure in UFS/FFS. Approved by: so Security: FreeBSD-SA-19:10.ufs Security: CVE-2019-5601 --- sbin/fsck_ffs/dir.c | 109 +++++++++++++++++++++++++++++++++------ sbin/fsck_ffs/fsck.h | 1 + sbin/fsck_ffs/fsck_ffs.8 | 7 ++- sbin/fsck_ffs/globs.c | 1 + sbin/fsck_ffs/main.c | 6 ++- sys/ufs/ufs/dir.h | 10 ++-- sys/ufs/ufs/ufs_lookup.c | 36 ++++++++----- 7 files changed, 132 insertions(+), 38 deletions(-) diff --git a/sbin/fsck_ffs/dir.c b/sbin/fsck_ffs/dir.c index 604349779c4..8efbabd1d7b 100644 --- a/sbin/fsck_ffs/dir.c +++ b/sbin/fsck_ffs/dir.c @@ -145,14 +145,23 @@ fsck_readdir(struct inodesc *idesc) struct direct *dp, *ndp; struct bufarea *bp; long size, blksiz, fix, dploc; + int dc; blksiz = idesc->id_numfrags * sblock.fs_fsize; bp = getdirblk(idesc->id_blkno, blksiz); if (idesc->id_loc % DIRBLKSIZ == 0 && idesc->id_filesize > 0 && idesc->id_loc < blksiz) { dp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc); - if (dircheck(idesc, dp)) + if ((dc = dircheck(idesc, dp)) > 0) { + if (dc == 2) { + /* + * dircheck() cleared unused directory space. + * Mark the buffer as dirty to write it out. + */ + dirty(bp); + } goto dpok; + } if (idesc->id_fix == IGNORE) return (0); fix = dofix(idesc, "DIRECTORY CORRUPTED"); @@ -179,19 +188,26 @@ fsck_readdir(struct inodesc *idesc) if ((idesc->id_loc % DIRBLKSIZ) == 0) return (dp); ndp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc); - if (idesc->id_loc < blksiz && idesc->id_filesize > 0 && - dircheck(idesc, ndp) == 0) { - size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ); - idesc->id_loc += size; - idesc->id_filesize -= size; - if (idesc->id_fix == IGNORE) - return (0); - fix = dofix(idesc, "DIRECTORY CORRUPTED"); - bp = getdirblk(idesc->id_blkno, blksiz); - dp = (struct direct *)(bp->b_un.b_buf + dploc); - dp->d_reclen += size; - if (fix) + if (idesc->id_loc < blksiz && idesc->id_filesize > 0) { + if ((dc = dircheck(idesc, ndp)) == 0) { + size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ); + idesc->id_loc += size; + idesc->id_filesize -= size; + if (idesc->id_fix == IGNORE) + return (0); + fix = dofix(idesc, "DIRECTORY CORRUPTED"); + bp = getdirblk(idesc->id_blkno, blksiz); + dp = (struct direct *)(bp->b_un.b_buf + dploc); + dp->d_reclen += size; + if (fix) + dirty(bp); + } else if (dc == 2) { + /* + * dircheck() cleared unused directory space. + * Mark the buffer as dirty to write it out. + */ dirty(bp); + } } return (dp); } @@ -199,6 +215,11 @@ fsck_readdir(struct inodesc *idesc) /* * Verify that a directory entry is valid. * This is a superset of the checks made in the kernel. + * Also optionally clears padding and unused directory space. + * + * Returns 0 if the entry is bad, 1 if the entry is good and no changes + * were made, and 2 if the entry is good but modified to clear out padding + * and unused space and needs to be written back to disk. */ static int dircheck(struct inodesc *idesc, struct direct *dp) @@ -207,15 +228,39 @@ dircheck(struct inodesc *idesc, struct direct *dp) char *cp; u_char type; u_int8_t namlen; - int spaceleft; + int spaceleft, modified, unused; + modified = 0; spaceleft = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ); if (dp->d_reclen == 0 || dp->d_reclen > spaceleft || - (dp->d_reclen & 0x3) != 0) + (dp->d_reclen & (DIR_ROUNDUP - 1)) != 0) goto bad; - if (dp->d_ino == 0) - return (1); + if (dp->d_ino == 0) { + /* + * Special case of an unused directory entry. Normally + * the kernel would coalesce unused space with the previous + * entry by extending its d_reclen, but there are situations + * (e.g. fsck) where that doesn't occur. + * If we're clearing out directory cruft (-z flag), then make + * sure this entry gets fully cleared as well. + */ + if (zflag && fswritefd >= 0) { + if (dp->d_type != 0) { + dp->d_type = 0; + modified = 1; + } + if (dp->d_namlen != 0) { + dp->d_namlen = 0; + modified = 1; + } + if (dp->d_name[0] != '\0') { + dp->d_name[0] = '\0'; + modified = 1; + } + } + goto good; + } size = DIRSIZ(0, dp); namlen = dp->d_namlen; type = dp->d_type; @@ -229,7 +274,37 @@ dircheck(struct inodesc *idesc, struct direct *dp) goto bad; if (*cp != '\0') goto bad; + +good: + if (zflag && fswritefd >= 0) { + /* + * Clear unused directory entry space, including the d_name + * padding. + */ + /* First figure the number of pad bytes. */ + unused = roundup2(namlen + 1, DIR_ROUNDUP) - (namlen + 1); + + /* Add in the free space to the end of the record. */ + unused += dp->d_reclen - DIRSIZ(0, dp); + + /* + * Now clear out the unused space, keeping track if we actually + * changed anything. + */ + for (cp = &dp->d_name[namlen + 1]; unused > 0; unused--, cp++) { + if (*cp != '\0') { + *cp = '\0'; + modified = 1; + } + } + + if (modified) { + return 2; + } + } + return (1); + bad: if (debug) printf("Bad dir: ino %d reclen %d namlen %d type %d name %s\n", diff --git a/sbin/fsck_ffs/fsck.h b/sbin/fsck_ffs/fsck.h index 12f863a719b..065048ec24e 100644 --- a/sbin/fsck_ffs/fsck.h +++ b/sbin/fsck_ffs/fsck.h @@ -310,6 +310,7 @@ extern ufs2_daddr_t bflag; /* location of alternate super block */ extern int debug; /* output debugging info */ extern int Eflag; /* delete empty data blocks */ extern int Zflag; /* zero empty data blocks */ +extern int zflag; /* zero unused directory space */ extern int inoopt; /* trim out unused inodes */ extern char ckclean; /* only do work if not cleanly unmounted */ extern int cvtlevel; /* convert to newer file system format */ diff --git a/sbin/fsck_ffs/fsck_ffs.8 b/sbin/fsck_ffs/fsck_ffs.8 index fa6f7d93a4d..b0c2ebb5fe5 100644 --- a/sbin/fsck_ffs/fsck_ffs.8 +++ b/sbin/fsck_ffs/fsck_ffs.8 @@ -29,7 +29,7 @@ .\" @(#)fsck.8 8.4 (Berkeley) 5/9/95 .\" $FreeBSD$ .\" -.Dd January 13, 2018 +.Dd May 3, 2019 .Dt FSCK_FFS 8 .Os .Sh NAME @@ -38,7 +38,7 @@ .Nd file system consistency check and interactive repair .Sh SYNOPSIS .Nm -.Op Fl BCdEFfnpRrSyZ +.Op Fl BCdEFfnpRrSyZz .Op Fl b Ar block .Op Fl c Ar level .Op Fl m Ar mode @@ -301,6 +301,9 @@ If both and .Fl Z are specified, blocks are first zeroed and then erased. +.It Fl z +Clear unused directory space. +The cleared space includes deleted file names and name padding. .El .Pp Inconsistencies checked are as follows: diff --git a/sbin/fsck_ffs/globs.c b/sbin/fsck_ffs/globs.c index a64224965f9..64edb12ae03 100644 --- a/sbin/fsck_ffs/globs.c +++ b/sbin/fsck_ffs/globs.c @@ -82,6 +82,7 @@ ufs2_daddr_t bflag; /* location of alternate super block */ int debug; /* output debugging info */ int Eflag; /* delete empty data blocks */ int Zflag; /* zero empty data blocks */ +int zflag; /* zero unused directory space */ int inoopt; /* trim out unused inodes */ char ckclean; /* only do work if not cleanly unmounted */ int cvtlevel; /* convert to newer file system format */ diff --git a/sbin/fsck_ffs/main.c b/sbin/fsck_ffs/main.c index 98cdafdd20f..244940158ab 100644 --- a/sbin/fsck_ffs/main.c +++ b/sbin/fsck_ffs/main.c @@ -86,7 +86,7 @@ main(int argc, char *argv[]) sync(); skipclean = 1; inoopt = 0; - while ((ch = getopt(argc, argv, "b:Bc:CdEfFm:npRrSyZ")) != -1) { + while ((ch = getopt(argc, argv, "b:Bc:CdEfFm:npRrSyZz")) != -1) { switch (ch) { case 'b': skipclean = 0; @@ -163,6 +163,10 @@ main(int argc, char *argv[]) Zflag++; break; + case 'z': + zflag++; + break; + default: usage(); } diff --git a/sys/ufs/ufs/dir.h b/sys/ufs/ufs/dir.h index 574a32f9e7f..9f79e8389af 100644 --- a/sys/ufs/ufs/dir.h +++ b/sys/ufs/ufs/dir.h @@ -105,13 +105,11 @@ struct direct { * The DIRSIZ macro gives the minimum record length which will hold * the directory entry. This requires the amount of space in struct direct * without the d_name field, plus enough space for the name with a terminating - * null byte (dp->d_namlen+1), rounded up to a 4 byte boundary. - * - * + * null byte (dp->d_namlen + 1), rounded up to a 4 byte boundary. */ -#define DIRECTSIZ(namlen) \ - ((__offsetof(struct direct, d_name) + \ - ((namlen)+1)*sizeof(((struct direct *)0)->d_name[0]) + 3) & ~3) +#define DIR_ROUNDUP 4 /* Directory name roundup size */ +#define DIRECTSIZ(namlen) \ + (roundup2(__offsetof(struct direct, d_name) + (namlen) + 1, DIR_ROUNDUP)) #if (BYTE_ORDER == LITTLE_ENDIAN) #define DIRSIZ(oldfmt, dp) \ ((oldfmt) ? DIRECTSIZ((dp)->d_type) : DIRECTSIZ((dp)->d_namlen)) diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c index ecbe09d687d..d6873af84b2 100644 --- a/sys/ufs/ufs/ufs_lookup.c +++ b/sys/ufs/ufs/ufs_lookup.c @@ -823,14 +823,21 @@ ufs_makedirentry(ip, cnp, newdirp) struct componentname *cnp; struct direct *newdirp; { + u_int namelen; -#ifdef INVARIANTS - if ((cnp->cn_flags & SAVENAME) == 0) - panic("ufs_makedirentry: missing name"); -#endif + namelen = (unsigned)cnp->cn_namelen; + KASSERT((cnp->cn_flags & SAVENAME) != 0, + ("ufs_makedirentry: missing name")); + KASSERT(namelen <= MAXNAMLEN, + ("ufs_makedirentry: name too long")); newdirp->d_ino = ip->i_number; - newdirp->d_namlen = cnp->cn_namelen; - bcopy(cnp->cn_nameptr, newdirp->d_name, (unsigned)cnp->cn_namelen + 1); + newdirp->d_namlen = namelen; + + /* Zero out after-name padding */ + *(u_int32_t *)(&newdirp->d_name[namelen & ~(DIR_ROUNDUP - 1)]) = 0; + + bcopy(cnp->cn_nameptr, newdirp->d_name, namelen); + if (ITOV(ip)->v_mount->mnt_maxsymlinklen > 0) newdirp->d_type = IFTODT(ip->i_mode); else { @@ -1209,16 +1216,21 @@ ufs_dirremove(dvp, ip, flags, isrmdir) if (ip && rep->d_ino != ip->i_number) panic("ufs_dirremove: ip %ju does not match dirent ino %ju\n", (uintmax_t)ip->i_number, (uintmax_t)rep->d_ino); - if (dp->i_count == 0) { - /* - * First entry in block: set d_ino to zero. - */ - ep->d_ino = 0; - } else { + /* + * Zero out the file directory entry metadata to reduce disk + * scavenging disclosure. + */ + bzero(&rep->d_name[0], rep->d_namlen); + rep->d_namlen = 0; + rep->d_type = 0; + rep->d_ino = 0; + + if (dp->i_count != 0) { /* * Collapse new free space into previous entry. */ ep->d_reclen += rep->d_reclen; + rep->d_reclen = 0; } #ifdef UFS_DIRHASH if (dp->i_dirhash != NULL) -- 2.45.0