From 6d2e2df764199f0a15fd743e79599391959cc17d Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 23 Nov 2018 22:24:59 +0000 Subject: [PATCH] Ensure that directory entry padding bytes are zeroed. Directory entries must be padded to maintain alignment; in many filesystems the padding was not initialized, resulting in stack memory being copied out to userspace. With the ino64 work there are also some explicit pad fields in struct dirent. Add a subroutine to clear these bytes and use it in the in-tree filesystems. The NFS client is omitted for now as it was fixed separately in r340787. Reported by: Thomas Barabosch, Fraunhofer FKIE Reviewed by: kib MFC after: 3 days Sponsored by: The FreeBSD Foundation --- .../opensolaris/uts/common/fs/zfs/zfs_ctldir.c | 6 ++++-- .../opensolaris/uts/common/fs/zfs/zfs_vnops.c | 1 + sys/fs/autofs/autofs_vnops.c | 11 ++++------- sys/fs/cd9660/cd9660_vnops.c | 2 +- sys/fs/devfs/devfs_devs.c | 2 +- sys/fs/ext2fs/ext2_lookup.c | 2 +- sys/fs/fdescfs/fdesc_vnops.c | 3 ++- sys/fs/fuse/fuse_internal.c | 2 +- sys/fs/msdosfs/msdosfs_vnops.c | 6 ++++-- sys/fs/nandfs/nandfs_vnops.c | 6 +++--- sys/fs/pseudofs/pseudofs_vnops.c | 2 +- sys/fs/smbfs/smbfs_io.c | 4 ++-- sys/fs/tmpfs/tmpfs_subr.c | 8 ++++---- sys/fs/tmpfs/tmpfs_vfsops.c | 2 +- sys/fs/tmpfs/tmpfs_vnops.c | 2 +- sys/fs/udf/udf_vnops.c | 5 +++-- sys/kern/uipc_mqueue.c | 2 +- sys/kern/vfs_export.c | 2 +- sys/sys/dirent.h | 13 +++++++++++++ sys/ufs/ufs/ufs_vnops.c | 2 +- 20 files changed, 50 insertions(+), 33 deletions(-) diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c index afe82346640..b59546f83e7 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c @@ -262,9 +262,9 @@ sfs_readdir_common(uint64_t parent_id, uint64_t id, struct vop_readdir_args *ap, entry.d_fileno = id; entry.d_type = DT_DIR; entry.d_name[0] = '.'; - entry.d_name[1] = '\0'; entry.d_namlen = 1; entry.d_reclen = sizeof(entry); + dirent_terminate(&entry); error = vfs_read_dirent(ap, &entry, uio->uio_offset); if (error != 0) return (SET_ERROR(error)); @@ -277,9 +277,9 @@ sfs_readdir_common(uint64_t parent_id, uint64_t id, struct vop_readdir_args *ap, entry.d_type = DT_DIR; entry.d_name[0] = '.'; entry.d_name[1] = '.'; - entry.d_name[2] = '\0'; entry.d_namlen = 2; entry.d_reclen = sizeof(entry); + dirent_terminate(&entry); error = vfs_read_dirent(ap, &entry, uio->uio_offset); if (error != 0) return (SET_ERROR(error)); @@ -694,6 +694,7 @@ zfsctl_root_readdir(ap) strcpy(entry.d_name, node->snapdir->sn_name); entry.d_namlen = strlen(entry.d_name); entry.d_reclen = sizeof(entry); + dirent_terminate(&entry); error = vfs_read_dirent(ap, &entry, uio->uio_offset); if (error != 0) { if (error == ENAMETOOLONG) @@ -1099,6 +1100,7 @@ zfsctl_snapdir_readdir(ap) entry.d_reclen = sizeof(entry); /* NOTE: d_off is the offset for the *next* entry. */ entry.d_off = cookie + dots_offset; + dirent_terminate(&entry); error = vfs_read_dirent(ap, &entry, uio->uio_offset); if (error != 0) { if (error == ENAMETOOLONG) diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c index b752ef5807c..604e0d5e277 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c @@ -2547,6 +2547,7 @@ zfs_readdir(vnode_t *vp, uio_t *uio, cred_t *cr, int *eofp, int *ncookies, u_lon next = &odp->d_off; (void) strlcpy(odp->d_name, zap.za_name, odp->d_namlen + 1); odp->d_type = type; + dirent_terminate(odp); odp = (dirent64_t *)((intptr_t)odp + reclen); } outcount += reclen; diff --git a/sys/fs/autofs/autofs_vnops.c b/sys/fs/autofs/autofs_vnops.c index 1c70d879179..09d77b9fccf 100644 --- a/sys/fs/autofs/autofs_vnops.c +++ b/sys/fs/autofs/autofs_vnops.c @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -44,7 +45,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -354,14 +354,11 @@ autofs_readdir_one(struct uio *uio, const char *name, int fileno, size_t *reclenp) { struct dirent dirent; - size_t namlen, padded_namlen, reclen; + size_t namlen, reclen; int error; namlen = strlen(name); - padded_namlen = roundup2(namlen + 1, __alignof(struct dirent)); - KASSERT(padded_namlen <= MAXNAMLEN, ("%zd > MAXNAMLEN", padded_namlen)); - reclen = offsetof(struct dirent, d_name) + padded_namlen; - + reclen = _GENERIC_DIRLEN(namlen); if (reclenp != NULL) *reclenp = reclen; @@ -376,7 +373,7 @@ autofs_readdir_one(struct uio *uio, const char *name, int fileno, dirent.d_type = DT_DIR; dirent.d_namlen = namlen; memcpy(dirent.d_name, name, namlen); - memset(dirent.d_name + namlen, 0, padded_namlen - namlen); + dirent_terminate(&dirent); error = uiomove(&dirent, reclen, uio); return (error); diff --git a/sys/fs/cd9660/cd9660_vnops.c b/sys/fs/cd9660/cd9660_vnops.c index ca291c7c5a3..27a186964a8 100644 --- a/sys/fs/cd9660/cd9660_vnops.c +++ b/sys/fs/cd9660/cd9660_vnops.c @@ -380,8 +380,8 @@ iso_uiodir(idp,dp,off) { int error; - dp->d_name[dp->d_namlen] = 0; dp->d_reclen = GENERIC_DIRSIZ(dp); + dirent_terminate(dp); if (idp->uio->uio_resid < dp->d_reclen) { idp->eofflag = 0; diff --git a/sys/fs/devfs/devfs_devs.c b/sys/fs/devfs/devfs_devs.c index 7295998279e..984ff274844 100644 --- a/sys/fs/devfs/devfs_devs.c +++ b/sys/fs/devfs/devfs_devs.c @@ -226,7 +226,7 @@ devfs_newdirent(char *name, int namelen) de->de_dirent->d_namlen = namelen; de->de_dirent->d_reclen = GENERIC_DIRSIZ(&d); bcopy(name, de->de_dirent->d_name, namelen); - de->de_dirent->d_name[namelen] = '\0'; + dirent_terminate(de->de_dirent); vfs_timestamp(&de->de_ctime); de->de_mtime = de->de_atime = de->de_ctime; de->de_links = 1; diff --git a/sys/fs/ext2fs/ext2_lookup.c b/sys/fs/ext2fs/ext2_lookup.c index d5fc6f93542..3d101a41502 100644 --- a/sys/fs/ext2fs/ext2_lookup.c +++ b/sys/fs/ext2fs/ext2_lookup.c @@ -223,9 +223,9 @@ ext2_readdir(struct vop_readdir_args *ap) dstdp.d_fileno = dp->e2d_ino; dstdp.d_reclen = GENERIC_DIRSIZ(&dstdp); bcopy(dp->e2d_name, dstdp.d_name, dstdp.d_namlen); - dstdp.d_name[dstdp.d_namlen] = '\0'; /* NOTE: d_off is the offset of the *next* entry. */ dstdp.d_off = offset + dp->e2d_reclen; + dirent_terminate(&dstdp); if (dstdp.d_reclen > uio->uio_resid) { if (uio->uio_resid == startresid) error = EINVAL; diff --git a/sys/fs/fdescfs/fdesc_vnops.c b/sys/fs/fdescfs/fdesc_vnops.c index d1731938f06..127bdccd8c4 100644 --- a/sys/fs/fdescfs/fdesc_vnops.c +++ b/sys/fs/fdescfs/fdesc_vnops.c @@ -561,8 +561,8 @@ fdesc_readdir(struct vop_readdir_args *ap) dp->d_namlen = i + 1; dp->d_reclen = UIO_MX; bcopy("..", dp->d_name, dp->d_namlen); - dp->d_name[i + 1] = '\0'; dp->d_type = DT_DIR; + dirent_terminate(dp); break; default: if (fdp->fd_ofiles[fcnt].fde_file == NULL) @@ -572,6 +572,7 @@ fdesc_readdir(struct vop_readdir_args *ap) dp->d_type = (fmp->flags & FMNT_LINRDLNKF) == 0 ? DT_CHR : DT_LNK; dp->d_fileno = i + FD_DESC; + dirent_terminate(dp); break; } /* NOTE: d_off is the offset of the *next* entry. */ diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 5982ccc7eaf..548602e2d2b 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -357,7 +357,7 @@ fuse_internal_readdir_processdata(struct uio *uio, memcpy((char *)cookediov->base + sizeof(struct dirent) - MAXNAMLEN - 1, (char *)buf + FUSE_NAME_OFFSET, fudge->namelen); - ((char *)cookediov->base)[bytesavail - 1] = '\0'; + dirent_terminate(de); err = uiomove(cookediov->base, cookediov->len, uio); if (err) { diff --git a/sys/fs/msdosfs/msdosfs_vnops.c b/sys/fs/msdosfs/msdosfs_vnops.c index e6172a25f6c..8bd94466142 100644 --- a/sys/fs/msdosfs/msdosfs_vnops.c +++ b/sys/fs/msdosfs/msdosfs_vnops.c @@ -1550,16 +1550,18 @@ msdosfs_readdir(struct vop_readdir_args *ap) switch (n) { case 0: dirbuf.d_namlen = 1; - strcpy(dirbuf.d_name, "."); + dirbuf.d_name[0] = '.'; break; case 1: dirbuf.d_namlen = 2; - strcpy(dirbuf.d_name, ".."); + dirbuf.d_name[0] = '.'; + dirbuf.d_name[1] = '.'; break; } dirbuf.d_reclen = GENERIC_DIRSIZ(&dirbuf); /* NOTE: d_off is the offset of the *next* entry. */ dirbuf.d_off = offset + sizeof(struct direntry); + dirent_terminate(&dirbuf); if (uio->uio_resid < dirbuf.d_reclen) goto out; error = uiomove(&dirbuf, dirbuf.d_reclen, uio); diff --git a/sys/fs/nandfs/nandfs_vnops.c b/sys/fs/nandfs/nandfs_vnops.c index 2b9f5364f69..ca6929e569f 100644 --- a/sys/fs/nandfs/nandfs_vnops.c +++ b/sys/fs/nandfs/nandfs_vnops.c @@ -1226,7 +1226,6 @@ nandfs_readdir(struct vop_readdir_args *ap) ndirent = (struct nandfs_dir_entry *)pos; name_len = ndirent->name_len; - memset(&dirent, 0, sizeof(struct dirent)); dirent.d_fileno = ndirent->inode; if (dirent.d_fileno) { dirent.d_type = ndirent->file_type; @@ -1235,6 +1234,7 @@ nandfs_readdir(struct vop_readdir_args *ap) dirent.d_reclen = GENERIC_DIRSIZ(&dirent); /* NOTE: d_off is the offset of the *next* entry. */ dirent.d_off = diroffset + ndirent->rec_len; + dirent_terminate(&dirent); DPRINTF(READDIR, ("copying `%*.*s`\n", name_len, name_len, dirent.d_name)); } @@ -1243,12 +1243,12 @@ nandfs_readdir(struct vop_readdir_args *ap) * If there isn't enough space in the uio to return a * whole dirent, break off read */ - if (uio->uio_resid < GENERIC_DIRSIZ(&dirent)) + if (uio->uio_resid < dirent.d_reclen) break; /* Transfer */ if (dirent.d_fileno) - uiomove(&dirent, GENERIC_DIRSIZ(&dirent), uio); + uiomove(&dirent, dirent.d_reclen, uio); /* Advance */ diroffset += ndirent->rec_len; diff --git a/sys/fs/pseudofs/pseudofs_vnops.c b/sys/fs/pseudofs/pseudofs_vnops.c index 9f918c103d8..b58791fd75e 100644 --- a/sys/fs/pseudofs/pseudofs_vnops.c +++ b/sys/fs/pseudofs/pseudofs_vnops.c @@ -828,7 +828,6 @@ pfs_readdir(struct vop_readdir_args *va) /* PFS_DELEN was picked to fit PFS_NAMLEN */ for (i = 0; i < PFS_NAMELEN - 1 && pn->pn_name[i] != '\0'; ++i) pfsent->entry.d_name[i] = pn->pn_name[i]; - pfsent->entry.d_name[i] = 0; pfsent->entry.d_namlen = i; /* NOTE: d_off is the offset of the *next* entry. */ pfsent->entry.d_off = offset + PFS_DELEN; @@ -855,6 +854,7 @@ pfs_readdir(struct vop_readdir_args *va) panic("%s has unexpected node type: %d", pn->pn_name, pn->pn_type); } PFS_TRACE(("%s", pfsent->entry.d_name)); + dirent_terminate(&pfsent->entry); STAILQ_INSERT_TAIL(&lst, pfsent, link); offset += PFS_DELEN; resid -= PFS_DELEN; diff --git a/sys/fs/smbfs/smbfs_io.c b/sys/fs/smbfs/smbfs_io.c index fd5aafaa562..82f73ceb459 100644 --- a/sys/fs/smbfs/smbfs_io.c +++ b/sys/fs/smbfs/smbfs_io.c @@ -106,8 +106,8 @@ smbfs_readvdir(struct vnode *vp, struct uio *uio, struct ucred *cred) de.d_namlen = offset + 1; de.d_name[0] = '.'; de.d_name[1] = '.'; - de.d_name[offset + 1] = '\0'; de.d_type = DT_DIR; + dirent_terminate(&de); error = uiomove(&de, DE_SIZE, uio); if (error) goto out; @@ -156,7 +156,7 @@ smbfs_readvdir(struct vnode *vp, struct uio *uio, struct ucred *cred) de.d_type = (ctx->f_attr.fa_attr & SMB_FA_DIR) ? DT_DIR : DT_REG; de.d_namlen = ctx->f_nmlen; bcopy(ctx->f_name, de.d_name, de.d_namlen); - de.d_name[de.d_namlen] = '\0'; + dirent_terminate(&de); if (smbfs_fastlookup) { error = smbfs_nget(vp->v_mount, vp, ctx->f_name, ctx->f_nmlen, &ctx->f_attr, &newvp); diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c index c0983430ed2..f3305841673 100644 --- a/sys/fs/tmpfs/tmpfs_subr.c +++ b/sys/fs/tmpfs/tmpfs_subr.c @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -50,7 +51,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -1120,8 +1120,8 @@ tmpfs_dir_getdotdent(struct tmpfs_node *node, struct uio *uio) dent.d_type = DT_DIR; dent.d_namlen = 1; dent.d_name[0] = '.'; - dent.d_name[1] = '\0'; dent.d_reclen = GENERIC_DIRSIZ(&dent); + dirent_terminate(&dent); if (dent.d_reclen > uio->uio_resid) error = EJUSTRETURN; @@ -1164,8 +1164,8 @@ tmpfs_dir_getdotdotdent(struct tmpfs_node *node, struct uio *uio) dent.d_namlen = 2; dent.d_name[0] = '.'; dent.d_name[1] = '.'; - dent.d_name[2] = '\0'; dent.d_reclen = GENERIC_DIRSIZ(&dent); + dirent_terminate(&dent); if (dent.d_reclen > uio->uio_resid) error = EJUSTRETURN; @@ -1285,8 +1285,8 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, int maxcookies, d.d_namlen = de->td_namelen; MPASS(de->td_namelen < sizeof(d.d_name)); (void)memcpy(d.d_name, de->ud.td_name, de->td_namelen); - d.d_name[de->td_namelen] = '\0'; d.d_reclen = GENERIC_DIRSIZ(&d); + dirent_terminate(&d); /* Stop reading if the directory entry we are treating is * bigger than the amount of data that can be returned. */ diff --git a/sys/fs/tmpfs/tmpfs_vfsops.c b/sys/fs/tmpfs/tmpfs_vfsops.c index 9447af6367f..68664f77552 100644 --- a/sys/fs/tmpfs/tmpfs_vfsops.c +++ b/sys/fs/tmpfs/tmpfs_vfsops.c @@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -56,7 +57,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c index 41f5a19efe2..6ff2c5da3a0 100644 --- a/sys/fs/tmpfs/tmpfs_vnops.c +++ b/sys/fs/tmpfs/tmpfs_vnops.c @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -51,7 +52,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include diff --git a/sys/fs/udf/udf_vnops.c b/sys/fs/udf/udf_vnops.c index 30558cf8693..b1b004f9516 100644 --- a/sys/fs/udf/udf_vnops.c +++ b/sys/fs/udf/udf_vnops.c @@ -843,10 +843,10 @@ udf_readdir(struct vop_readdir_args *a) dir.d_fileno = node->hash_id; dir.d_type = DT_DIR; dir.d_name[0] = '.'; - dir.d_name[1] = '\0'; dir.d_namlen = 1; dir.d_reclen = GENERIC_DIRSIZ(&dir); dir.d_off = 1; + dirent_terminate(&dir); uiodir.dirent = &dir; error = udf_uiodir(&uiodir, dir.d_reclen, uio, 1); if (error) @@ -856,10 +856,10 @@ udf_readdir(struct vop_readdir_args *a) dir.d_type = DT_DIR; dir.d_name[0] = '.'; dir.d_name[1] = '.'; - dir.d_name[2] = '\0'; dir.d_namlen = 2; dir.d_reclen = GENERIC_DIRSIZ(&dir); dir.d_off = 2; + dirent_terminate(&dir); uiodir.dirent = &dir; error = udf_uiodir(&uiodir, dir.d_reclen, uio, 2); } else { @@ -870,6 +870,7 @@ udf_readdir(struct vop_readdir_args *a) DT_DIR : DT_UNKNOWN; dir.d_reclen = GENERIC_DIRSIZ(&dir); dir.d_off = ds->this_off; + dirent_terminate(&dir); uiodir.dirent = &dir; error = udf_uiodir(&uiodir, dir.d_reclen, uio, ds->this_off); diff --git a/sys/kern/uipc_mqueue.c b/sys/kern/uipc_mqueue.c index 2f5cad6466d..3fb04f29767 100644 --- a/sys/kern/uipc_mqueue.c +++ b/sys/kern/uipc_mqueue.c @@ -1428,7 +1428,6 @@ mqfs_readdir(struct vop_readdir_args *ap) entry.d_fileno = pn->mn_fileno; for (i = 0; i < MQFS_NAMELEN - 1 && pn->mn_name[i] != '\0'; ++i) entry.d_name[i] = pn->mn_name[i]; - entry.d_name[i] = 0; entry.d_namlen = i; switch (pn->mn_type) { case mqfstype_root: @@ -1447,6 +1446,7 @@ mqfs_readdir(struct vop_readdir_args *ap) panic("%s has unexpected node type: %d", pn->mn_name, pn->mn_type); } + dirent_terminate(&entry); if (entry.d_reclen > uio->uio_resid) break; if (offset >= uio->uio_offset) { diff --git a/sys/kern/vfs_export.c b/sys/kern/vfs_export.c index 231b77e0ac2..669d4e9fa3b 100644 --- a/sys/kern/vfs_export.c +++ b/sys/kern/vfs_export.c @@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$"); #include "opt_inet6.h" #include +#include #include #include #include @@ -55,7 +56,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include diff --git a/sys/sys/dirent.h b/sys/sys/dirent.h index a54dc07d489..5bc6cf49afe 100644 --- a/sys/sys/dirent.h +++ b/sys/sys/dirent.h @@ -126,6 +126,19 @@ struct freebsd11_dirent { #ifdef _KERNEL #define GENERIC_DIRSIZ(dp) _GENERIC_DIRSIZ(dp) + +/* + * Ensure that padding bytes are zeroed and that the name is NUL-terminated. + */ +static inline void +dirent_terminate(struct dirent *dp) +{ + + dp->d_pad0 = 0; + dp->d_pad1 = 0; + memset(dp->d_name + dp->d_namlen, 0, + dp->d_reclen - (__offsetof(struct dirent, d_name) + dp->d_namlen)); +} #endif #endif /* !_SYS_DIRENT_H_ */ diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 18462f0a71c..e2c28fb054c 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -2217,9 +2217,9 @@ ufs_readdir(ap) dstdp.d_fileno = dp->d_ino; dstdp.d_reclen = GENERIC_DIRSIZ(&dstdp); bcopy(dp->d_name, dstdp.d_name, dstdp.d_namlen); - dstdp.d_name[dstdp.d_namlen] = '\0'; /* NOTE: d_off is the offset of the *next* entry. */ dstdp.d_off = offset + dp->d_reclen; + dirent_terminate(&dstdp); if (dstdp.d_reclen > uio->uio_resid) { if (uio->uio_resid == startresid) error = EINVAL; -- 2.45.0