From fb0466c92925dedf5573525b075c09892eae861e Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Fri, 26 May 2006 16:56:34 +0000 Subject: [PATCH] * Fix a number of cases where too much kernel memory might be allocated to satisfy a directory read operation. * Calculate a minimum of (1) allocated directory cookie and limit the maximum to 1024. * Rewrite ufs_readdir() (part 1/2) to use the buffer cache instead of allocating a kernel buffer and to do better validation of the scanned directory entries. * Use a simpler fix for EXT2FS. Reported-by: [NetBSD.org #7471] --- sys/vfs/gnu/ext2fs/ext2_lookup.c | 14 ++- sys/vfs/isofs/cd9660/cd9660_vnops.c | 14 ++- sys/vfs/msdosfs/msdosfs_vnops.c | 6 +- sys/vfs/procfs/procfs_vnops.c | 17 ++- sys/vfs/udf/udf_vnops.c | 6 +- sys/vfs/ufs/ufs_vnops.c | 170 +++++++++++++++++----------- 6 files changed, 141 insertions(+), 86 deletions(-) diff --git a/sys/vfs/gnu/ext2fs/ext2_lookup.c b/sys/vfs/gnu/ext2fs/ext2_lookup.c index fa3ddd0b15..09e5c11fd1 100644 --- a/sys/vfs/gnu/ext2fs/ext2_lookup.c +++ b/sys/vfs/gnu/ext2fs/ext2_lookup.c @@ -5,7 +5,7 @@ * University of Utah, Department of Computer Science * * $FreeBSD: src/sys/gnu/ext2fs/ext2_lookup.c,v 1.21.2.3 2002/11/17 02:02:42 bde Exp $ - * $DragonFly: src/sys/vfs/gnu/ext2fs/ext2_lookup.c,v 1.22 2006/05/06 02:43:13 dillon Exp $ + * $DragonFly: src/sys/vfs/gnu/ext2fs/ext2_lookup.c,v 1.23 2006/05/26 16:56:18 dillon Exp $ */ /* * Copyright (c) 1989, 1993 @@ -159,6 +159,8 @@ ext2_readdir(struct vop_readdir_args *ap) count -= (uio->uio_offset + count) & (DIRBLKSIZ -1); if (count <= 0) count += DIRBLKSIZ; + if (count > MAXBSIZE) /* limit to a reasonable size */ + count = MAXBSIZE; #ifdef EXT2FS_DEBUG printf("ext2_readdir: uio_offset = %lld, uio_resid = %d, count = %d\n", @@ -219,8 +221,14 @@ ext2_readdir(struct vop_readdir_args *ap) if (uio->uio_segflg != UIO_SYSSPACE || uio->uio_iovcnt != 1) panic("ext2fs_readdir: unexpected uio from NFS server"); - MALLOC(cookies, u_long *, ncookies * sizeof(u_long), M_TEMP, - M_WAITOK); + if (ncookies) { + MALLOC(cookies, u_long *, + ncookies * sizeof(u_long), + M_TEMP, M_WAITOK); + } else { + MALLOC(cookies, u_long *, + sizeof(u_long), M_TEMP, M_WAITOK); + } off = startoffset; for (dp = (struct ext2_dir_entry_2 *)dirbuf, cookiep = cookies, ecookies = cookies + ncookies; diff --git a/sys/vfs/isofs/cd9660/cd9660_vnops.c b/sys/vfs/isofs/cd9660/cd9660_vnops.c index ca979fbbbe..883fe69d2c 100644 --- a/sys/vfs/isofs/cd9660/cd9660_vnops.c +++ b/sys/vfs/isofs/cd9660/cd9660_vnops.c @@ -37,7 +37,7 @@ * * @(#)cd9660_vnops.c 8.19 (Berkeley) 5/27/95 * $FreeBSD: src/sys/isofs/cd9660/cd9660_vnops.c,v 1.62 1999/12/15 23:01:51 eivind Exp $ - * $DragonFly: src/sys/vfs/isofs/cd9660/cd9660_vnops.c,v 1.25 2006/05/06 02:43:13 dillon Exp $ + * $DragonFly: src/sys/vfs/isofs/cd9660/cd9660_vnops.c,v 1.26 2006/05/26 16:56:21 dillon Exp $ */ #include @@ -495,11 +495,15 @@ cd9660_readdir(struct vop_readdir_args *ap) idp->cookies = NULL; } else { /* - * Guess the number of cookies needed. + * Guess the number of cookies needed. Guess at least + * 1 to avoid a degenerate case in malloc, and cap at + * a reasonable limit. */ - ncookies = uio->uio_resid / 16; - MALLOC(cookies, u_long *, ncookies * sizeof(u_int), M_TEMP, - M_WAITOK); + ncookies = uio->uio_resid / 16 + 1; + if (ncookies > 1024) + ncookies = 1024; + MALLOC(cookies, u_long *, ncookies * sizeof(u_int), + M_TEMP, M_WAITOK); idp->cookies = cookies; idp->ncookies = ncookies; } diff --git a/sys/vfs/msdosfs/msdosfs_vnops.c b/sys/vfs/msdosfs/msdosfs_vnops.c index 21eaaf7be6..215fa75d0b 100644 --- a/sys/vfs/msdosfs/msdosfs_vnops.c +++ b/sys/vfs/msdosfs/msdosfs_vnops.c @@ -1,5 +1,5 @@ /* $FreeBSD: src/sys/msdosfs/msdosfs_vnops.c,v 1.95.2.4 2003/06/13 15:05:47 trhodes Exp $ */ -/* $DragonFly: src/sys/vfs/msdosfs/msdosfs_vnops.c,v 1.37 2006/05/06 02:43:14 dillon Exp $ */ +/* $DragonFly: src/sys/vfs/msdosfs/msdosfs_vnops.c,v 1.38 2006/05/26 16:56:22 dillon Exp $ */ /* $NetBSD: msdosfs_vnops.c,v 1.68 1998/02/10 14:10:04 mrg Exp $ */ /*- @@ -1596,7 +1596,9 @@ msdosfs_readdir(struct vop_readdir_args *ap) return (EINVAL); if (ap->a_ncookies) { - ncookies = uio->uio_resid / 16; + ncookies = uio->uio_resid / 16 + 1; + if (ncookies > 1024) + ncookies = 1024; MALLOC(cookies, u_long *, ncookies * sizeof(u_long), M_TEMP, M_WAITOK); *ap->a_cookies = cookies; diff --git a/sys/vfs/procfs/procfs_vnops.c b/sys/vfs/procfs/procfs_vnops.c index fe5deaf9c3..310112fd56 100644 --- a/sys/vfs/procfs/procfs_vnops.c +++ b/sys/vfs/procfs/procfs_vnops.c @@ -37,7 +37,7 @@ * @(#)procfs_vnops.c 8.18 (Berkeley) 5/21/95 * * $FreeBSD: src/sys/miscfs/procfs/procfs_vnops.c,v 1.76.2.7 2002/01/22 17:22:59 nectar Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_vnops.c,v 1.32 2006/05/24 18:59:51 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_vnops.c,v 1.33 2006/05/26 16:56:31 dillon Exp $ */ /* @@ -847,7 +847,9 @@ procfs_readdir_proc(struct vop_readdir_args *ap) return(0); error = 0; - i = uio->uio_offset; + i = (int)uio->uio_offset; + if (i < 0) + return (EINVAL); for (pt = &proc_targets[i]; !error && uio->uio_resid > 0 && i < nproc_targets; pt++, i++) { @@ -861,7 +863,7 @@ procfs_readdir_proc(struct vop_readdir_args *ap) break; } - uio->uio_offset = i; + uio->uio_offset = (off_t)i; return(0); } @@ -884,11 +886,14 @@ procfs_readdir_root(struct vop_readdir_args *ap) int res; info.error = 0; - info.i = uio->uio_offset; + info.i = (int)uio->uio_offset; + + if (info.i < 0) + return (EINVAL); + info.pcnt = 0; info.uio = uio; info.cred = ap->a_cred; - while (info.pcnt < 3) { res = procfs_readdir_root_callback(NULL, &info); if (res < 0) @@ -896,7 +901,7 @@ procfs_readdir_root(struct vop_readdir_args *ap) } if (res >= 0) allproc_scan(procfs_readdir_root_callback, &info); - uio->uio_offset = info.i; + uio->uio_offset = (off_t)info.i; return (info.error); } diff --git a/sys/vfs/udf/udf_vnops.c b/sys/vfs/udf/udf_vnops.c index 47e8cabe84..7a28f624a7 100644 --- a/sys/vfs/udf/udf_vnops.c +++ b/sys/vfs/udf/udf_vnops.c @@ -24,7 +24,7 @@ * SUCH DAMAGE. * * $FreeBSD: src/sys/fs/udf/udf_vnops.c,v 1.33 2003/12/07 05:04:49 scottl Exp $ - * $DragonFly: src/sys/vfs/udf/udf_vnops.c,v 1.21 2006/05/06 02:43:14 dillon Exp $ + * $DragonFly: src/sys/vfs/udf/udf_vnops.c,v 1.22 2006/05/26 16:56:32 dillon Exp $ */ /* udf_vnops.c */ @@ -708,7 +708,9 @@ udf_readdir(struct vop_readdir_args *a) * function will be called again and thing will pick up were * it left off. */ - ncookies = uio->uio_resid / 8; + ncookies = uio->uio_resid / 8 + 1; + if (ncookies > 1024) + ncookies = 1024; cookies = malloc(sizeof(u_long) * ncookies, M_TEMP, M_WAITOK); uiodir.ncookies = ncookies; uiodir.cookies = cookies; diff --git a/sys/vfs/ufs/ufs_vnops.c b/sys/vfs/ufs/ufs_vnops.c index 95bbb7e324..075a25b830 100644 --- a/sys/vfs/ufs/ufs_vnops.c +++ b/sys/vfs/ufs/ufs_vnops.c @@ -37,7 +37,7 @@ * * @(#)ufs_vnops.c 8.27 (Berkeley) 5/27/95 * $FreeBSD: src/sys/ufs/ufs/ufs_vnops.c,v 1.131.2.8 2003/01/02 17:26:19 bde Exp $ - * $DragonFly: src/sys/vfs/ufs/ufs_vnops.c,v 1.48 2006/05/06 02:43:14 dillon Exp $ + * $DragonFly: src/sys/vfs/ufs/ufs_vnops.c,v 1.49 2006/05/26 16:56:34 dillon Exp $ */ #include "opt_quota.h" @@ -1636,12 +1636,6 @@ ufs_symlink(struct vop_old_symlink_args *ap) /* * Vnode op for reading directories. * - * The routine below assumes that the on-disk format of a directory - * is the same as that defined by . If the on-disk - * format changes, then it will be necessary to do a conversion - * from the on-disk format that read returns to the format defined - * by . - * * ufs_readdir(struct vnode *a_vp, struct uio *a_uio, struct ucred *a_cred, * int *a_eofflag, int *ncookies, u_long **a_cookies) */ @@ -1651,47 +1645,78 @@ ufs_readdir(struct vop_readdir_args *ap) { struct uio *uio = ap->a_uio; struct vnode *vp = ap->a_vp; - struct direct *edp, *dp; - struct uio auio; - struct iovec aiov; - caddr_t dirbuf; - int readcnt, retval; - int count, error; + struct direct *dp; + struct buf *bp; + int retval; + int error; + int offset; /* offset into buffer cache buffer */ + int eoffset; /* end of buffer clipped to file EOF */ + int pickup; /* pickup point */ int ncookies; - off_t startoffset = uio->uio_offset; + int cookie_index; + u_long *cookies; - count = uio->uio_resid; + if (uio->uio_offset < 0) + return (EINVAL); /* - * Avoid complications for partial directory entries by adjusting - * the i/o to end at a block boundary. Don't give up (like the old ufs - * does) if the initial adjustment gives a negative count, since - * many callers don't supply a large enough buffer. The correct - * size is a little larger than DIRBLKSIZ to allow for expansion - * of directory entries, but some callers just use 512. + * Guess the number of cookies needed. Make sure we compute at + * least 1, and no more then a reasonable limit. */ - count -= (uio->uio_offset + count) & (DIRBLKSIZ -1); - if (count <= 0) - count += DIRBLKSIZ; - - auio = *uio; - auio.uio_iov = &aiov; - auio.uio_iovcnt = 1; - auio.uio_resid = count; - auio.uio_segflg = UIO_SYSSPACE; - aiov.iov_len = count; - MALLOC(dirbuf, caddr_t, count, M_TEMP, M_WAITOK); - aiov.iov_base = dirbuf; - error = VOP_READ(vp, &auio, 0, ap->a_cred); - if (error == 0) { - readcnt = count - auio.uio_resid; - edp = (struct direct *)&dirbuf[readcnt]; - ncookies = 0; - for (dp = (struct direct *)dirbuf; - !error && uio->uio_resid > 0 && dp < edp; ) { - if (dp->d_reclen <= 0) { + if (ap->a_ncookies) { + ncookies = uio->uio_resid / 16 + 1; + if (ncookies > 1024) + ncookies = 1024; + cookies = malloc(ncookies * sizeof(u_long), M_TEMP, M_WAITOK); + } else { + ncookies = -1; /* force conditionals below */ + cookies = NULL; + } + cookie_index = 0; + + /* + * Past or at EOF + */ + if (uio->uio_offset >= VTOI(vp)->i_size) { + if (ap->a_eofflag) + *ap->a_eofflag = 1; + if (ap->a_ncookies) { + *ap->a_ncookies = cookie_index; + *ap->a_cookies = cookies; + } + return(0); + } + + /* + * Loop until we run out of cookies, we run out of user buffer, + * or we hit the directory EOF. + * + * Always start scans at the beginning of the buffer, don't trust + * the offset supplied by userland. + */ + while ((error = UFS_BLKATOFF(vp, uio->uio_offset, NULL, &bp)) == 0) { + pickup = (int)(uio->uio_offset - bp->b_loffset); + offset = 0; + retval = 0; + if (bp->b_loffset + bp->b_bcount > VTOI(vp)->i_size) + eoffset = (int)(VTOI(vp)->i_size - bp->b_loffset); + else + eoffset = bp->b_bcount; + + while (offset < eoffset) { + dp = (struct direct *)(bp->b_data + offset); + if (dp->d_reclen <= 0 || (dp->d_reclen & 3) || + offset + dp->d_reclen > bp->b_bcount) { error = EIO; break; } + if (offsetof(struct direct, d_name[dp->d_namlen]) > dp->d_reclen) { + error = EIO; + break; + } + if (offset < pickup) { + offset += dp->d_reclen; + continue; + } #if BYTE_ORDER == LITTLE_ENDIAN if (OFSFMT(vp)) { retval = vop_write_dirent(&error, uio, @@ -1704,40 +1729,49 @@ ufs_readdir(struct vop_readdir_args *ap) dp->d_ino, dp->d_type, dp->d_namlen, dp->d_name); } - if (retval) break; - /* advance dp */ - dp = (struct direct *)((char *)dp + dp->d_reclen); - if (!error) - ncookies++; - } - /* we need to correct uio_offset */ - uio->uio_offset = startoffset + (caddr_t)dp - dirbuf; - - if (!error && ap->a_ncookies != NULL) { - u_long *cookiep, *cookies, *ecookies; - off_t off; - - if (uio->uio_segflg != UIO_SYSSPACE || uio->uio_iovcnt != 1) - panic("ufs_readdir: unexpected uio from NFS server"); - MALLOC(cookies, u_long *, ncookies * sizeof(u_long), M_TEMP, - M_WAITOK); - off = startoffset; - for (dp = (struct direct *)dirbuf, - cookiep = cookies, ecookies = cookies + ncookies; - cookiep < ecookies; - dp = (struct direct *)((caddr_t) dp + dp->d_reclen)) { - off += dp->d_reclen; - *cookiep++ = (u_long) off; + if (cookies) { + cookies[cookie_index] = + (u_long)bp->b_loffset + offset; } - *ap->a_ncookies = ncookies; - *ap->a_cookies = cookies; + ++cookie_index; + offset += dp->d_reclen; + if (cookie_index == ncookies) + break; + } + + /* + * This will align the next loop to the beginning of the + * next block, and pickup will calculate to 0. + */ + uio->uio_offset = bp->b_loffset + offset; + brelse(bp); + + if (retval || error || cookie_index == ncookies || + uio->uio_offset >= VTOI(vp)->i_size) { + break; } } - FREE(dirbuf, M_TEMP); if (ap->a_eofflag) *ap->a_eofflag = VTOI(vp)->i_size <= uio->uio_offset; + + /* + * Report errors only if we didn't manage to read anything + */ + if (error && cookie_index == 0) { + if (cookies) { + free(cookies, M_TEMP); + *ap->a_ncookies = 0; + *ap->a_cookies = NULL; + } + } else { + error = 0; + if (cookies) { + *ap->a_ncookies = cookie_index; + *ap->a_cookies = cookies; + } + } return (error); } -- 2.41.0