[LibOS] Fix poll() crashing on pseudo-files

LibOS/shim/include/shim_fs.h

@@ -706,5 +706,6 @@ ssize_t str_write(struct shim_handle* hdl, const void* buf, size_t count);
 off_t str_seek(struct shim_handle* hdl, off_t offset, int whence);
 int str_flush(struct shim_handle* hdl);
 int str_truncate(struct shim_handle* hdl, off_t len);
+off_t str_poll(struct shim_handle* hdl, int poll_type);
 
 #endif /* _SHIM_FS_H_ */

LibOS/shim/src/fs/shim_fs_pseudo.c

@@ -442,25 +442,26 @@ static int pseudo_close(struct shim_handle* hdl) {
     }
 }
 
-/* TODO: add support for polling TYPE_STR handles; currently `shim_do_poll` doesn't call this for
- * anything else than TYPE_STR and TYPE_DEV */
 static off_t pseudo_poll(struct shim_handle* hdl, int poll_type) {
-    if (poll_type == FS_POLL_SZ)
-        return 0;
-
     assert(hdl->dentry);
     struct pseudo_node* node = pseudo_find(hdl->dentry);
     if (!node)
         return -ENOENT;
     switch (node->type) {
         case PSEUDO_DEV: {
+            if (poll_type == FS_POLL_SZ)
+                return 0;
+
             off_t ret = 0;
             if ((poll_type & FS_POLL_RD) && node->dev.dev_ops.read)
                 ret |= FS_POLL_RD;
             if ((poll_type & FS_POLL_WR) && node->dev.dev_ops.write)
                 ret |= FS_POLL_WR;
             return ret;
         }
+        case PSEUDO_STR:
+            return str_poll(hdl, poll_type);
+
         default:
             return -ENOSYS;
     }

LibOS/shim/src/fs/str/fs.c

@@ -258,13 +258,38 @@ int str_truncate(struct shim_handle* hdl, off_t len) {
     return ret;
 }
 
+off_t str_poll(struct shim_handle* hdl, int poll_type) {
+    assert(hdl->type == TYPE_STR);
+
+    struct shim_str_handle* strhdl = &hdl->info.str;
+    struct shim_str_data* data = strhdl->data;
+    assert(data);
+
+    if (poll_type == FS_POLL_SZ)
+        return data->len;
+
+    off_t ret = 0;
+    if (poll_type & FS_POLL_RD) {
+        if (data->len > 0) {
+            assert(data->str);
+            if (!strhdl->ptr || strhdl->ptr < (data->str + data->len))
+                ret |= FS_POLL_RD;
+        }
+    }
+    if (poll_type & FS_POLL_WR)
+        ret |= FS_POLL_WR;
+
+    return ret;
+}
+
 struct shim_fs_ops str_fs_ops = {
     .close    = &str_close,
     .read     = &str_read,
     .write    = &str_write,
     .seek     = &str_seek,
     .flush    = &str_flush,
     .truncate = &str_truncate,
+    .poll     = &str_poll,
 };
 
 struct shim_d_ops str_d_ops = {

LibOS/shim/src/sys/shim_poll.c

@@ -102,9 +102,12 @@ static long _shim_do_poll(struct pollfd* fds, nfds_t nfds, int timeout_ms) {
             continue;
         }
 
-        if (hdl->type == TYPE_FILE || hdl->type == TYPE_DEV) {
-            /* Files and devs are special cases: their poll is emulated at LibOS level; do not
-             * include them in handles-to-poll array but instead use handle-specific callback. */
+        if (hdl->type == TYPE_FILE || hdl->type == TYPE_DEV || hdl->type == TYPE_STR) {
+            /* Files, devs and strings are special cases: their poll is emulated at LibOS level; do
+             * not include them in handles-to-poll array but instead use handle-specific
+             * callback.
+             *
+             * TODO: we probably should use the poll() callback in all cases. */
             int shim_events = 0;
             if ((fds[i].events & (POLLIN | POLLRDNORM)) && (hdl->acc_mode & MAY_READ))
                 shim_events |= FS_POLL_RD;
@@ -119,6 +122,13 @@ static long _shim_do_poll(struct pollfd* fds, nfds_t nfds, int timeout_ms) {
             if (shim_revents & FS_POLL_WR)
                 fds[i].revents |= fds[i].events & (POLLOUT | POLLWRNORM);
 
+            if (fds[i].revents)
+                nrevents++;
+            continue;
+        }
+
+        if (!hdl->pal_handle) {
+            fds[i].revents = POLLNVAL;
             nrevents++;
             continue;
         }

LibOS/shim/test/regression/poll.c

@@ -1,44 +1,116 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* Copyright (C) 2021 Intel Corporation
+ *                    Paweł Marczewski <pawel@invisiblethingslab.com>
+ */
+
+/* Simple sanity check for poll() on various file types. */
+
+#include <err.h>
+#include <errno.h>
+#include <fcntl.h>
 #include <poll.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 
-int main(void) {
+static void check_poll(int fd, short events, int revents) {
+    struct pollfd fds[] = {
+        {.fd = fd, .events = events, .revents = 0},
+    };
+
+    int ret = poll(fds, /*nfds=*/1, /*timeout=*/0);
+    fprintf(stderr, "poll {events = 0x%x} = %d", events, ret);
+    if (ret == 1)
+        fprintf(stderr, "; {revents = 0x%x}", (int)fds[0].revents);
+    fprintf(stderr, "\n");
+    fflush(stderr);
+    if (ret < 0)
+        err(1, "poll");
+
+    int expected = revents ? 1 : 0;
+    if (ret != expected)
+        errx(1, "expected poll to return %d\n", expected);
+
+    if (ret == 1 && fds[0].revents != revents)
+        errx(1, "expected revents to be 0x%x", revents);
+}
+
+static void write_byte(int fd, char c) {
+    ssize_t n;
+    do {
+        char c = 0;
+        n = write(fd, &c, sizeof(c));
+    } while (n == -1 && errno == EINTR);
+    if (n == -1)
+        err(1, "write");
+    if (n != 1)
+        errx(1, "write returned %ld", n);
+}
+
+static void test_pipe(void) {
+    printf("testing poll() on pipe...\n");
     int ret;
-    int fd[2];
-    char string[] = "Hello, world!\n";
 
-    ret = pipe(fd);
-    if (ret < 0) {
-        perror("pipe creation failed");
-        return 1;
-    }
+    int fds[2];
+    ret = pipe(fds);
+    if (ret < 0)
+        err(1, "pipe");
 
-    struct pollfd outfds[] = {
-        {.fd = fd[1], .events = POLLOUT},
-    };
-    ret = poll(outfds, 1, -1);
-    if (ret <= 0) {
-        perror("poll with POLLOUT failed");
-        return 1;
-    }
-    printf("poll(POLLOUT) returned %d file descriptors\n", ret);
+    check_poll(fds[1], POLLOUT, POLLOUT);
+    check_poll(fds[0], POLLIN, 0);
 
-    struct pollfd infds[] = {
-        {.fd = fd[0], .events = POLLIN},
-    };
-    size_t size = strlen(string) + 1;
-    if (write(fd[1], string, size) != size) {
-        perror("write error");
-        return 1;
-    }
-    ret = poll(infds, 1, -1);
-    if (ret <= 0) {
-        perror("poll with POLLIN failed");
-        return 1;
+    write_byte(fds[1], 0);
+
+    check_poll(fds[0], POLLIN, POLLIN);
+
+    if (close(fds[0]) < 0 || close(fds[1]) < 0)
+        err(1, "close");
+}
+
+static void test_file(const char* path, int flags, int events1, int revents1, int events2,
+                      int revents2) {
+    printf("testing poll() on %s...\n", path);
+
+    int fd = open(path, flags, 0600);
+    if (fd < 0)
+        err(1, "open");
+
+    if (events1)
+        check_poll(fd, events1, revents1);
+
+    if (events2)
+        check_poll(fd, events2, revents2);
+
+    if (close(fd) < 0)
+        err(1, "close");
+
+    if (flags & O_CREAT) {
+        if (unlink(path) < 0)
+            err(1, "unlink");
     }
-    printf("poll(POLLIN) returned %d file descriptors\n", ret);
+}
+
+int main(int argc, char** argv) {
+    setbuf(stdout, NULL);
+    setbuf(stderr, NULL);
+
+    test_pipe();
+
+    /* Emulated device */
+    test_file("/dev/null", O_RDWR, POLLIN, POLLIN, POLLOUT, POLLOUT);
+
+    /* File in /proc/ */
+    test_file("/proc/meminfo", O_RDONLY, POLLIN, POLLIN, 0, 0);
+
+    /* Host file */
+    test_file(argv[0], O_RDONLY, POLLIN, POLLIN, 0, 0);
+
+    /* Host file (empty) */
+    test_file("tmp/host_file", O_RDWR | O_CREAT | O_TRUNC, POLLIN, 0, POLLOUT, POLLOUT);
 
+    printf("TEST OK\n");
     return 0;
 }

LibOS/shim/test/regression/test_libos.py

@@ -824,9 +824,12 @@ def test_011_epoll_epollet(self):
         self.assertIn('TEST OK', stdout)
 
     def test_020_poll(self):
-        stdout, _ = self.run_binary(['poll'])
-        self.assertIn('poll(POLLOUT) returned 1 file descriptors', stdout)
-        self.assertIn('poll(POLLIN) returned 1 file descriptors', stdout)
+        try:
+            stdout, _ = self.run_binary(['poll'])
+        finally:
+            if os.path.exists("tmp/host_file"):
+                os.remove("tmp/host_file")
+        self.assertIn('TEST OK', stdout)
 
     def test_021_poll_many_types(self):
         stdout, _ = self.run_binary(['poll_many_types'])