0.17.0 --- an enumeration ends with its directory, a context has the stack it asks for, and input can arrive unaltered (#37)

Fixes #34. Supersedes #35, whose commit is carried with its author.

- **An enumeration ends when its directory is closed** (#35), and `kal_fs_list_next` refuses a closed directory or an iterator that is not an open listing of the directory given, without reading through it (clause 6.7). A close that races a read of the same enumeration marks the listing and the reader releases it (clause 6.6).
- **A started context's stack** is 8 MiB unless more is asked for, with a 256 KiB inaccessible guard beneath it; the guard exceeds the 69,008-byte frame measured in the C library above at `-O0`. `kal_task_start_sized` (openkal 0.16) gives at least the size asked for and never less than the default.
- **`KAL_TERM_PASS_INPUT`** is `ISTRIP | INLCR | IGNCR | ICRNL`: read as set only where all four are clear; set, all four are cleared; clear, `ICRNL` is restored.

Observed in this package's tests: the leak through `/proc/self/fd`, the refusals, two hundred close/read races; a pseudo terminal delivering the return key and Ctrl+J as 0x0d and 0x0a with the position set; a least stack size as room for 1, 16 and 64 MiB. The specification's suite at `openkal-0.16.0`: 201 held, 0 did not hold.

Author: Sunrisepeak

README.md

@@ -5,10 +5,10 @@ for Linux, written on the kernel's own system-call interface.
 
 ```toml
 [dependencies]
-openkal = "0.15.0"
+openkal = "0.16.0"
 
 [target.'cfg(os = "linux")'.dependencies]
-openkal-linux = "0.16.1"
+openkal-linux = "0.17.0"
 ```
 
 ## Why it does not use a C library
@@ -45,6 +45,22 @@ second and third.
 All eight. `tools/check-surface.sh --complete` in the specification package
 compares the exported names against `SURFACE.txt`.
 
+## The stack a started context is given
+
+Where this implementation creates a context (the `standalone` arrangement) it
+maps the stack itself: 8 MiB unless more is asked for, which is the size the
+host's own C library gives a thread, and a 256 KiB inaccessible guard beneath
+it. The mapping is committed as it is touched, so the size costs address space
+rather than memory. The guard is larger than one page because a frame of 69,008
+bytes was measured in the C library above at `-O0`, and a frame larger than the
+guard steps over it.
+
+`kal_task_start_sized` (openkal 0.16) gives at least the size asked for and
+never less than the default, so a C library that asks for its own smaller
+default on every thread does not shrink what each one had. A request larger than
+the address space is `kal_err_no_memory`. Where the program carries a runtime of
+its own, the request is passed to that runtime.
+
 ## The region a context stands on
 
 `kal_task_stack` reports the stack of the calling context, and this

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "openkal-linux"
-version     = "0.16.1"
+version     = "0.17.0"
 description = "The reference implementation of openkal for Linux, written on the kernel's own system-call interface so that it can be placed beneath a C library as well as above one."
 license     = "Apache-2.0"
 
@@ -63,7 +63,7 @@ provides-interfaces = [
 ]
 
 [dependencies]
-openkal = "0.15.0"
+openkal = "0.16.0"
 
 # The package contributes definitions and no modules. The interface it
 # implements is declared by the specification package, which this package

src/fs.cpp

@@ -254,11 +254,98 @@ kal_uintptr put_name(const char* src, kal_uintptr n,
 // calling.
 struct listing {
     int      fd;
+    int      dir;      // the descriptor of the directory it was begun upon
+    listing* next;
+    int      busy;     // a context is inside kal_fs_list_next with it
+    int      doomed;   // its directory was closed while it was busy
     okl_uptr used;
     okl_uptr pos;
     char     buf[8192];
 };
 
+// THE ENUMERATIONS NOT YET READ TO THE END, BECAUSE CLOSING THE DIRECTORY MUST
+// END THEM. fs.h releases an iterator "by reading past the end or by closing
+// the directory that produced it", and the descriptor a listing holds is its
+// own, so closing the directory's descriptor does not reach it. Measured before
+// this list existed: a C library above that opened a directory larger than its
+// buffer, read one entry and closed it, 64 times, was left holding 64 more
+// descriptors.
+//
+// The lock spins rather than suspends, because suspending would make openkal.fs
+// depend upon openkal.task, as src/memory.cpp says of its own.
+listing*     g_listings = nullptr;
+volatile int g_listings_held = 0;
+
+void listings_lock() {
+    while (__atomic_exchange_n(&g_listings_held, 1, __ATOMIC_ACQUIRE)) okl::sys(okl::nr_sched_yield);
+}
+void listings_unlock() { __atomic_store_n(&g_listings_held, 0, __ATOMIC_RELEASE); }
+
+void listing_release(listing* s) {
+    okl::sys(okl::nr_close, s->fd);
+    kal_free(s, sizeof(listing), alignof(listing));
+}
+
+// Ends every listing begun upon `dir'. What is ended is unlinked under the lock
+// and released after it.
+//
+// A LISTING ANOTHER CONTEXT IS READING IS NOT RELEASED HERE. Clause 6.6 requires
+// that concurrent operations upon one handle not damage this implementation's
+// state, and releasing the buffer a getdents64 in another context is writing
+// into would do exactly that. Such a listing is marked, and the context reading
+// it releases it on the way out; it can no longer be found, so nothing reaches
+// it after that.
+void listings_end(int dir) {
+    listing* done = nullptr;
+    listings_lock();
+    for (listing** p = &g_listings; *p != nullptr;) {
+        listing* s = *p;
+        if (s->dir != dir || s->doomed) { p = &s->next; continue; }
+        if (s->busy) { s->doomed = 1; p = &s->next; continue; }
+        *p = s->next; s->next = done; done = s;
+    }
+    listings_unlock();
+    while (done != nullptr) { listing* s = done; done = s->next; listing_release(s); }
+}
+
+// Takes `s' for the calling context, or reports that it is not a listing of
+// `dir' that is still open.
+//
+// THE WORD IS FOUND RATHER THAN BELIEVED. An iterator is the caller's copy of an
+// address this implementation allocated, and closing its directory frees what
+// it named --- so a caller that kept it, or that raced the close, holds an
+// address that may by now be anything. Searching the open listings for it is
+// what lets kal_fs_list_next refuse such a word without reading through it. The
+// search is over enumerations not yet read to their end, which a program holds
+// a handful of.
+bool listing_take(listing* s, int dir) {
+    for (;;) {
+        listings_lock();
+        listing* p = g_listings;
+        while (p != nullptr && p != s) p = p->next;
+        if (p == nullptr || p->dir != dir || p->doomed) { listings_unlock(); return false; }
+        if (!p->busy) { p->busy = 1; listings_unlock(); return true; }
+        // Two contexts reading one enumeration take turns; the order in which
+        // they receive its entries is the unspecified order clause 6.6 permits.
+        listings_unlock();
+        okl::sys(okl::nr_sched_yield);
+    }
+}
+
+// Gives `s' back, and releases it if it reached its end or its directory was
+// closed while it was being read.
+void listing_give(listing* s, bool ended) {
+    listings_lock();
+    s->busy = 0;
+    const bool release = ended || s->doomed;
+    if (release) {
+        for (listing** p = &g_listings; *p != nullptr; p = &(*p)->next)
+            if (*p == s) { *p = s->next; break; }
+    }
+    listings_unlock();
+    if (release) listing_release(s);
+}
+
 }  // namespace
 
 extern "C" {
@@ -312,7 +399,11 @@ int kal_fs_open(kal_dir base, const char* name, kal_uintptr len,
 
 void kal_fs_close_dir(kal_dir d) {
     const int fd = okl::unpack(d.h);
-    if (fd >= 0) { okl::retire(d.h); okl::sys(okl::nr_close, fd); }
+    if (fd < 0) return;
+    // Before the close, because after it the number may already name another
+    // directory whose enumerations are not this one's.
+    listings_end(fd);
+    okl::retire(d.h); okl::sys(okl::nr_close, fd);
 }
 
 void kal_fs_close_file(kal_file f) {
@@ -515,25 +606,31 @@ int kal_fs_list_begin(kal_dir d, kal_uintptr* iter) {
     if (okl::failed(own)) return okl::translate(own);
     auto* s = static_cast<listing*>(kal_alloc(sizeof(listing), alignof(listing)));
     if (s == nullptr) { okl::sys(okl::nr_close, own); return kal_err_no_memory; }
-    s->fd = static_cast<int>(own); s->used = 0; s->pos = 0;
+    s->fd = static_cast<int>(own); s->dir = fd; s->busy = 0; s->doomed = 0;
+    s->used = 0; s->pos = 0;
+    listings_lock(); s->next = g_listings; g_listings = s; listings_unlock();
     *iter = reinterpret_cast<kal_uintptr>(s);
     return kal_ok;
 }
 
-int kal_fs_list_next(kal_dir, kal_uintptr* iter,
+int kal_fs_list_next(kal_dir d, kal_uintptr* iter,
                      char* name_out, kal_uintptr name_cap,
                      kal_uintptr* name_len, int* kind) {
-    if (iter == nullptr || *iter == 0) return kal_err_invalid;
+    // A CLOSED DIRECTORY IS A RELEASED HANDLE (clause 6.7), and closing it
+    // released the iterator too, which fs.h states. Both are refused here
+    // without the iterator being read through.
+    const int fd = okl::unpack(d.h);
+    if (fd < 0 || iter == nullptr || *iter == 0) return kal_err_invalid;
     auto* s = reinterpret_cast<listing*>(*iter);
+    if (!listing_take(s, fd)) return kal_err_invalid;
     for (;;) {
         if (s->pos >= s->used) {
             const okl_long r = okl::sys(okl::nr_getdents64, s->fd,
                                         reinterpret_cast<okl_long>(s->buf),
                                         static_cast<okl_long>(sizeof s->buf));
             if (okl::interrupted(r)) continue;
             if (okl::failed(r) || r == 0) {
-                okl::sys(okl::nr_close, s->fd);
-                kal_free(s, sizeof(listing), alignof(listing));
+                listing_give(s, true);
                 *iter = 0;
                 if (name_len) *name_len = 0;
                 return okl::failed(r) ? okl::translate(r) : kal_ok;
@@ -551,6 +648,7 @@ int kal_fs_list_next(kal_dir, kal_uintptr* iter,
         if (kind) *kind = e->type == okl::dt_dir ? kal_node_directory
                         : e->type == okl::dt_reg ? kal_node_file
                         : e->type == okl::dt_lnk ? kal_node_link : kal_node_other;
+        listing_give(s, false);
         return kal_ok;
     }
 }

src/sys.h

@@ -534,6 +534,14 @@ enum : okl_u32 {
     t_isig = 0000001u, t_icanon = 0000002u, t_echo = 0000010u, t_iexten = 0100000u,
 };
 enum : okl_u32 { t_ixon = 0002000u };
+// `t_istrip', `t_inlcr', `t_igncr' and `t_icrnl' are the mechanisms by which
+// this kernel alters input on its way: clearing the high bit, rewriting a line
+// feed into a carriage return, discarding a carriage return, and rewriting a
+// carriage return into a line feed. KAL_TERM_PASS_INPUT is the whole of them,
+// for the reason given above for the three that keep a keystroke. The last is
+// the one this kernel applies ordinarily, and the one that made the return key
+// and Ctrl+J the same byte (mcpplibs/openkal#48).
+enum : okl_u32 { t_istrip = 0000040u, t_inlcr = 0000100u, t_igncr = 0000200u, t_icrnl = 0000400u };
 enum { v_time = 5, v_min = 6 };
 
 // --- openkal.net and openkal.datagram --------------------------------------