From d08f717585ab19a81dfc5bd8aed1ac4edcb2b157 Mon Sep 17 00:00:00 2001 From: markj 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 afe823466407..b59546f83e72 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 b752ef5807c8..604e0d5e2775 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 1c70d8791795..09d77b9fccf8 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 ca291c7c5a3f..27a186964a80 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 7295998279e7..984ff2748441 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 d5fc6f93542d..3d101a415024 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 d1731938f062..127bdccd8c40 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 5982ccc7eafd..548602e2d2bf 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 e6172a25f6c0..8bd944661420 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 2b9f5364f69b..ca6929e569f7 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 9f918c103d87..b58791fd75eb 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 fd5aafaa5623..82f73ceb4594 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 c0983430ed2a..f3305841673d 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 9447af6367fe..68664f775527 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 41f5a19efe2a..6ff2c5da3a0f 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 30558cf86931..b1b004f95166 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 2f5cad6466d8..3fb04f29767e 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 231b77e0ac2e..669d4e9fa3bf 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 a54dc07d489b..5bc6cf49afe7 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 18462f0a71ca..e2c28fb054cf 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;