[LibOS] Remove mmap logic on read/write in chroot FS

Previously, the LibOS logic in chroot FS's read/write emulation tried
to be (too) smart: it mmapped regular files and performed a memcpy on
the user-requested file offset + size. This mmap trick required the
use of DkStreamMap() which is inefficient on Linux-SGX PAL, since for
SGX, the PAL must memcpy from untrusted file buffer into the trusted
enclave buffer. Thus, reads/writes in chroot FS resulted in two
memcpys under SGX.

This commit removes this complicated mmap logic and simply calls
DkStreamRead/Write() for regular files. In other words, the mmap
optimization is moved out of the LibOS layer and into the PAL layer.

LibOS/shim/include/shim_handle.h

@@ -86,11 +86,6 @@ struct shim_file_handle {
     enum shim_file_type type;
     off_t size;
     off_t marker;
-
-    enum { FILEBUF_MAP, FILEBUF_NONE } buf_type;
-    size_t mapsize;
-    off_t mapoffset;
-    void* mapbuf;
 };
 
 #define FILE_HANDLE_DATA(hdl)  ((hdl)->info.file.data)

LibOS/shim/src/fs/chroot/fs.c

@@ -473,7 +473,6 @@ static int chroot_open (struct shim_handle * hdl, struct shim_dentry * dent,
     hdl->type       = TYPE_FILE;
     file->marker    = (flags & O_APPEND) ? size : 0;
     file->size      = size;
-    file->buf_type  = (data->type == FILE_REGULAR) ? FILEBUF_MAP : FILEBUF_NONE;
     hdl->flags      = flags;
     hdl->acc_mode   = ACC_MODE(flags & O_ACCMODE);
     qstrcopy(&hdl->uri, &data->host_uri);
@@ -503,7 +502,6 @@ static int chroot_creat (struct shim_handle * hdl, struct shim_dentry * dir,
     hdl->type       = TYPE_FILE;
     file->marker    = (flags & O_APPEND) ? size : 0;
     file->size      = size;
-    file->buf_type  = (data->type == FILE_REGULAR) ? FILEBUF_MAP : FILEBUF_NONE;
     hdl->flags      = flags;
     hdl->acc_mode   = ACC_MODE(flags & O_ACCMODE);
     qstrcopy(&hdl->uri, &data->host_uri);
@@ -617,7 +615,7 @@ static int chroot_hstat (struct shim_handle * hdl, struct stat * stat)
             stat->st_dev  = mdata ? (dev_t) mdata->ino_base : 0;
             stat->st_ino  = dent ? (ino_t) dent->ino : 0;
             stat->st_size = file->size;
-            stat->st_mode |= (file->buf_type == FILEBUF_MAP) ? S_IFREG : S_IFCHR;
+            stat->st_mode |= (file->type == FILE_REGULAR) ? S_IFREG : S_IFCHR;
         }
 
         return 0;
@@ -626,190 +624,18 @@ static int chroot_hstat (struct shim_handle * hdl, struct stat * stat)
     return query_dentry(hdl->dentry, hdl->pal_handle, NULL, stat);
 }
 
-static void chroot_flush_map(struct shim_handle* hdl) {
-    struct shim_file_handle* file = &hdl->info.file;
-
-    if (file->buf_type == FILEBUF_MAP) {
-        lock(&hdl->lock);
-        void* mapbuf = file->mapbuf;
-        size_t mapsize = file->mapsize;
-        file->mapoffset = 0;
-        file->mapbuf = NULL;
-        unlock(&hdl->lock);
-
-        if (mapbuf) {
-            DkStreamUnmap(mapbuf, mapsize);
-
-            if (bkeep_munmap(mapbuf, mapsize, VMA_INTERNAL) < 0)
-                BUG();
-        }
-    }
-}
-
 static int chroot_flush(struct shim_handle* hdl) {
     int ret = DkStreamFlush(hdl->pal_handle);
     if (ret < 0)
         return ret;
-
-    chroot_flush_map(hdl);
     return 0;
 }
 
 static int chroot_close(struct shim_handle* hdl) {
-    chroot_flush_map(hdl);
-    return 0;
-}
-
-static inline int __map_buffer (struct shim_handle * hdl, size_t size)
-{
-    struct shim_file_handle * file = &hdl->info.file;
-
-    if (file->mapbuf) {
-        if (file->marker >= file->mapoffset &&
-            file->marker + size <= file->mapoffset + file->mapsize)
-            return 0;
-
-        DkStreamUnmap(file->mapbuf, file->mapsize);
-
-        if (bkeep_munmap(file->mapbuf, file->mapsize, VMA_INTERNAL) < 0)
-            BUG();
-
-        file->mapbuf    = NULL;
-        file->mapoffset = 0;
-    }
-
-    /* second, reallocate the buffer */
-    size_t bufsize = file->mapsize ? : FILE_BUFMAP_SIZE;
-    assert(IS_POWER_OF_2(bufsize));
-    off_t  mapoff = ALIGN_DOWN_POW2(file->marker, bufsize);
-    size_t maplen = bufsize;
-    int flags = MAP_FILE | MAP_PRIVATE | VMA_INTERNAL;
-    int prot = PROT_READ;
-
-    if (hdl->acc_mode & MAY_WRITE) {
-        flags = MAP_FILE | MAP_SHARED | VMA_INTERNAL;
-        prot |= PROT_WRITE;
-    }
-
-    while (mapoff + maplen < file->marker + size)
-        maplen *= 2;
-
-    /* Create the bookkeeping before allocating the memory. */
-    void * mapbuf = bkeep_unmapped_any(maplen, prot, flags, mapoff, "filebuf");
-    if (!mapbuf)
-        return -ENOMEM;
-
-    PAL_PTR mapped = DkStreamMap(hdl->pal_handle, mapbuf, PAL_PROT(prot, flags),
-                                 mapoff, maplen);
-
-    if (!mapped) {
-        bkeep_munmap(mapbuf, maplen, flags);
-        return -PAL_ERRNO;
-    }
-
-    assert((void*)mapped == mapbuf);
-
-    file->mapbuf    = mapbuf;
-    file->mapoffset = mapoff;
-    file->mapsize   = maplen;
-
+    __UNUSED(hdl);
     return 0;
 }
 
-static ssize_t map_read (struct shim_handle * hdl, void * buf, size_t count)
-{
-    struct shim_file_handle * file = &hdl->info.file;
-    ssize_t ret = 0;
-    lock(&hdl->lock);
-
-    struct shim_file_data * data = FILE_HANDLE_DATA(hdl);
-    off_t size = atomic_read(&data->size);
-
-    if (check_version(hdl) &&
-        file->size < size)
-        file->size = size;
-
-    off_t marker = file->marker;
-
-    if (marker >= file->size) {
-        count = 0;
-        goto out;
-    }
-
-    if ((ret = __map_buffer(hdl, count)) < 0) {
-        unlock(&hdl->lock);
-        return ret;
-    }
-
-    size_t bytes_left;
-    if (!__builtin_sub_overflow(file->size, marker, &bytes_left) && bytes_left < count)
-        count = bytes_left;
-
-    if (count) {
-        memcpy(buf, file->mapbuf + (marker - file->mapoffset), count);
-        file->marker = marker + count;
-    }
-
-out:
-    unlock(&hdl->lock);
-    return count;
-}
-
-static ssize_t map_write (struct shim_handle * hdl, const void * buf, size_t count)
-{
-    struct shim_file_handle * file = &hdl->info.file;
-    ssize_t ret = 0;
-    lock(&hdl->lock);
-
-    struct shim_file_data * data = FILE_HANDLE_DATA(hdl);
-    off_t marker = file->marker;
-
-    off_t new_marker;
-    if (__builtin_add_overflow(marker, count, &new_marker)) {
-        // We can't handle this case reasonably.
-        ret = -EFBIG;
-        goto out;
-    }
-
-    if (new_marker > file->size) {
-        file->size = new_marker;
-
-        PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
-
-        if (pal_ret == PAL_STREAM_ERROR) {
-            ret = -PAL_ERRNO;
-            goto out;
-        }
-
-        if (pal_ret < count) {
-           file->size -= count - pal_ret;
-        }
-
-        chroot_update_size(hdl, file, data);
-
-        if (__builtin_add_overflow(marker, pal_ret, &file->marker)) {
-            // Should never happen. Even if it would, we couldn't recover from this condition.
-            BUG();
-        }
-        ret = (ssize_t) pal_ret;
-        goto out;
-    }
-
-    if ((ret = __map_buffer(hdl, count)) < 0)
-        goto out;
-
-
-    if (count) {
-        memcpy(file->mapbuf + (marker - file->mapoffset), buf, count);
-        file->marker = new_marker;
-    }
-
-    ret = count;
-out:
-    unlock(&hdl->lock);
-    return ret;
-}
-
 static ssize_t chroot_read (struct shim_handle * hdl, void * buf, size_t count)
 {
     ssize_t ret = 0;
@@ -834,16 +660,7 @@ static ssize_t chroot_read (struct shim_handle * hdl, void * buf, size_t count)
         goto out;
     }
 
-    if (file->buf_type == FILEBUF_MAP) {
-        ret = map_read(hdl, buf, count);
-        if (ret != -EACCES)
-            goto out;
-
-        lock(&hdl->lock);
-        file->buf_type = FILEBUF_NONE;
-    } else {
-        lock(&hdl->lock);
-    }
+    lock(&hdl->lock);
 
     PAL_NUM pal_ret = DkStreamRead(hdl->pal_handle, file->marker, count, buf, NULL, 0);
     if (pal_ret != PAL_STREAM_ERROR) {
@@ -884,16 +701,7 @@ static ssize_t chroot_write (struct shim_handle * hdl, const void * buf, size_t
         goto out;
     }
 
-    if (hdl->info.file.buf_type == FILEBUF_MAP) {
-        ret = map_write(hdl, buf, count);
-        if (ret != -EACCES)
-            goto out;
-
-        lock(&hdl->lock);
-        file->buf_type = FILEBUF_NONE;
-    } else {
-        lock(&hdl->lock);
-    }
+    lock(&hdl->lock);
 
     PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
     if (pal_ret != PAL_STREAM_ERROR) {
@@ -1186,9 +994,6 @@ static int chroot_checkout (struct shim_handle * hdl)
             hdl->pal_handle = NULL;
     }
 
-    hdl->info.file.mapsize = 0;
-    hdl->info.file.mapoffset = 0;
-    hdl->info.file.mapbuf = NULL;
     return 0;
 }
 
@@ -1268,7 +1073,7 @@ static off_t chroot_poll (struct shim_handle * hdl, int poll_type)
 
     off_t marker = file->marker;
 
-    if (file->buf_type == FILEBUF_MAP) {
+    if (file->type == FILE_REGULAR) {
         ret = poll_type & FS_POLL_WR;
         if ((poll_type & FS_POLL_RD) && file->size > marker)
             ret |= FS_POLL_RD;