libafs/dir: Verify directory pathnames

Provide a new routine, afs_dir_GetVerifiedBlob() which will ensure
that the pathname contained within a directory blob is correctly
terminated before returning it to the caller. For the purposes of this
function, correct termination is defined as having a terminating
\0 character within the same directory page as the blob itself.

Change-Id: I4b3bbb95cb49645a8ac52e6061f9e24f89924831
Reviewed-on: http://gerrit.openafs.org/5241
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c
index b61353e..ca1998a 100644 (file)
--- a/src/afs/LINUX/osi_vnodeops.c
+++ b/src/afs/LINUX/osi_vnodeops.c
@@ -340,25 +340,19 @@ afs_linux_readdir(struct file *fp, void *dirbuf, filldir_t filldir)
        if (!dirpos)
            break;
 
-       code = afs_dir_GetBlob(tdc, dirpos, &entry);
-       if (code)
-           break;
-       de = (struct DirEntry *)entry.data;
-
-       ino = afs_calc_inum(avc->f.fid.Cell, avc->f.fid.Fid.Volume,
-                           ntohl(de->fid.vnode));
-
-       if (de->name)
-           len = strlen(de->name);
-       else {
-           printf("afs_linux_readdir: afs_dir_GetBlob failed, null name (inode %lx, dirpos %d)\n", 
-                  (unsigned long)&tdc->f.inode, dirpos);
-           DRelease(&entry, 0);
+       code = afs_dir_GetVerifiedBlob(tdc, dirpos, &de);
+       if (code) {
+           afs_warn("Corrupt directory (inode %lx, dirpos %d)",
+                    (unsigned long)&tdc->f.inode, dirpos);
            ReleaseSharedLock(&avc->lock);
            afs_PutDCache(tdc);
            code = -ENOENT;
            goto out;
-       }
+        }
+
+       ino = afs_calc_inum (avc->f.fid.Cell, avc->f.fid.Fid.Volume,
+                            ntohl(de->fid.vnode));
+       len = strlen(de->name);
 
        /* filldir returns -EINVAL when the buffer is full. */
        {

diff --git a/src/afs/VNOPS/afs_vnop_readdir.c b/src/afs/VNOPS/afs_vnop_readdir.c
index 5aca352..f2f0a84 100644 (file)
--- a/src/afs/VNOPS/afs_vnop_readdir.c
+++ b/src/afs/VNOPS/afs_vnop_readdir.c
@@ -738,8 +738,12 @@ afs_readdir(OSI_VC_DECL(avc), struct uio *auio, afs_ucred_t *acred)
        origOffset = AFS_UIO_OFFSET(auio);
        /* scan for the next interesting entry scan for in-use blob otherwise up point at
         * this blob note that ode, if non-zero, also represents a held dir page */
-       if (!(us = BlobScan(tdc, (origOffset >> 5)))
-           || (afs_dir_GetBlob(tdc, us, &nextEntry) != 0)) {
+       us = BlobScan(tdc, (origOffset >> 5));
+
+       if (us)
+          afs_dir_GetVerifiedBlob(tdc, us, &nextEntry);
+
+       if (us == 0 || nde == NULL) {
            /* failed to setup nde, return what we've got, and release ode */
            if (len) {
                /* something to hand over. */

diff --git a/src/dir/dir.c b/src/dir/dir.c
index 28647d0..191d5bc 100644 (file)
--- a/src/dir/dir.c
+++ b/src/dir/dir.c
@@ -425,25 +425,24 @@ afs_dir_EnumerateDir(dir_file_t dir, int (*proc) (void *, char *name,
        num = ntohs(dhp->hashTable[i]);
        while (num != 0) {
            /* Walk down the hash table list. */
-           DErrno = 0;
-           if (afs_dir_GetBlob(dir, num, &entrybuf) != 0) {
-               if (DErrno) {
-                   /* we failed, return why */
-                   DRelease(&headerbuf, 0);
-                   return DErrno;
-               }
-               break;
-           }
+           code = afs_dir_GetVerifiedBlob(dir, num, &entrybuf);
+           if (code)
+               goto out;
+
            ep = (struct DirEntry *)entrybuf.data;
+           if (!ep)
+               break;
 
            num = ntohs(ep->next);
            code = (*proc) (hook, ep->name, ntohl(ep->fid.vnode),
                            ntohl(ep->fid.vunique));
            DRelease(&entrybuf, 0);
            if (code)
-               break;
+               goto out;
        }
     }
+
+out:
     DRelease(&headerbuf, 0);
     return 0;
 }
@@ -467,9 +466,9 @@ afs_dir_IsEmpty(dir_file_t dir)
        num = ntohs(dhp->hashTable[i]);
        while (num != 0) {
            /* Walk down the hash table list. */
-           if (afs_dir_GetBlob(dir, num, &entrybuf) != 0);
+           if (afs_dir_GetVerifiedBlob(dir, num, &entrybuf) != 0);
                break;
-           ep = (struct DirEntry *)entrybuf.data;
+
            if (strcmp(ep->name, "..") && strcmp(ep->name, ".")) {
                DRelease(&entrybuf, 0);
                DRelease(&headerbuf, 0);
@@ -483,22 +482,78 @@ afs_dir_IsEmpty(dir_file_t dir)
     return 0;
 }
 
-int
-afs_dir_GetBlob(dir_file_t dir, afs_int32 blobno, struct DirBuffer *buffer)
+/* Return a pointer to an entry, given its number. Also return the maximum
+ * size of the entry, which is determined by its position within the directory
+ * page.
+ */
+
+static int
+GetBlobWithLimit(dir_file_t dir, afs_int32 blobno,
+                struct DirBuffer *buffer, afs_size_t *maxlen)
 {
+    afs_size_t pos;
     int code;
 
+    *maxlen = 0;
     memset(buffer, 0, sizeof(struct DirBuffer));
 
     code = DRead(dir, blobno >> LEPP, buffer);
     if (code)
        return code;
 
-    buffer->data = (void *)(((long)buffer->data) + 32 * (blobno & (EPP - 1)));
+    pos = 32 * (blobno & (EPP - 1));
+
+    *maxlen = AFS_PAGESIZE - pos - 1;
+
+    buffer->data = (void *)(((char *)buffer->data) + pos);
 
     return 0;
 }
 
+/* Given an entries number, return a pointer to that entry */
+int
+afs_dir_GetBlob(dir_file_t dir, afs_int32 blobno, struct DirBuffer *buffer)
+{
+    afs_size_t maxlen = 0;
+    return GetBlobWithLimit(dir, blobno, buffer, &maxlen);
+}
+
+/* Return an entry, having verified that the name held within the entry
+ * doesn't overflow off the end of the directory page it is contained
+ * within
+ */
+
+int
+afs_dir_GetVerifiedBlob(dir_file_t file, afs_int32 blobno,
+                       struct DirBuffer *outbuf)
+{
+    struct DirEntry *dir;
+    struct DirBuffer buffer;
+    afs_size_t maxlen;
+    int code;
+    char *cp;
+
+    outbuf = NULL;
+
+    code = GetBlobWithLimit(file, blobno, &buffer, &maxlen);
+    if (code)
+       return code;
+
+    dir = (struct DirEntry *)buffer.data;
+
+    /* A blob is only valid if the name within it is NULL terminated before
+     * the end of the blob's containing page */
+    for (cp = dir->name; *cp != '\0' && cp < ((char *)dir) + maxlen; cp++);
+
+    if (*cp != '\0') {
+       DRelease(&buffer, 0);
+       return EIO;
+    }
+
+    *outbuf = buffer;
+    return 0;
+}
+
 int
 afs_dir_DirHash(char *string)
 {
@@ -550,8 +605,9 @@ FindItem(dir_file_t dir, char *ename, struct DirBuffer *prevbuf,
        return ENOENT;
     }
 
-    code = afs_dir_GetBlob(dir, (u_short) ntohs(dhp->hashTable[i]),
-                          &curr);
+    code = afs_dir_GetVerifiedBlob(dir,
+                                  (u_short) ntohs(dhp->hashTable[i]),
+                                  &curr);
     if (code) {
        DRelease(&prev, 0);
        return code;
@@ -569,22 +625,22 @@ FindItem(dir_file_t dir, char *ename, struct DirBuffer *prevbuf,
            return 0;
        }
        DRelease(&prev, 0);
-       if (tp->next == 0) {
-           /* The end of the line */
-           DRelease(&curr, 0);
-           return ENOENT;
-       }
 
        prev = curr;
        prev.data = &(tp->next);
 
-       code = afs_dir_GetBlob(dir, (u_short) ntohs(tp->next), &curr);
-       if (code) {
-           DRelease(&prev, 0);
-           return code;
-       }
+       if (tp->next == 0)
+           goto out; /* The end of the line */
+
+       code = afs_dir_GetVerifiedBlob(dir, (u_short) ntohs(tp->next),
+                                      &curr);
+       if (code)
+           goto out;
     }
-    /* Never reached */
+
+out:
+    DRelease(&prev, 0);
+    return ENOENT;
 }
 
 static int
@@ -611,8 +667,9 @@ FindFid (void *dir, afs_uint32 vnode, afs_uint32 unique,
 
     for (i=0; i<NHASHENT; i++) {
        if (dhp->hashTable[i] != 0) {
-           code = afs_dir_GetBlob(dir, (u_short)ntohs(dhp->hashTable[i]),
-                                  &curr);
+           code = afs_dir_GetVerifiedBlob(dir,
+                                          (u_short)ntohs(dhp->hashTable[i]),
+                                          &curr);
            if (code) {
                DRelease(&header, 0);
                return code;
@@ -634,7 +691,8 @@ FindFid (void *dir, afs_uint32 vnode, afs_uint32 unique,
                if (next == 0)
                    break;
 
-               code = afs_dir_GetBlob(dir, (u_short)ntohs(next), &curr);
+               code = afs_dir_GetVerifiedBlob(dir, (u_short)ntohs(next),
+                                              &curr);
                if (code) {
                    DRelease(&header, 0);
                    return code;

diff --git a/src/dir/dir.h b/src/dir/dir.h
index 3d1055a..f5c8eef 100644 (file)
--- a/src/dir/dir.h
+++ b/src/dir/dir.h
@@ -104,6 +104,8 @@ extern int afs_dir_EnumerateDir(dir_file_t dir,
 extern int afs_dir_IsEmpty(dir_file_t dir);
 extern int afs_dir_GetBlob(dir_file_t dir, afs_int32 blobno,
                           struct DirBuffer *);
+extern int afs_dir_GetVerifiedBlob(dir_file_t dir, afs_int32 blobno,
+                                  struct DirBuffer *);
 extern int afs_dir_DirHash(char *string);
 
 extern int afs_dir_InverseLookup (void *dir, afs_uint32 vnode,