[kernel] Add automatic verification to syscalls

Since we have a DSL for specifying syscalls, we can create a verificaton
method for each syscall that can cover most argument (and eventually
capability) verification instead of doing it piecemeal in each syscall
implementation, which can be more error-prone.

Now a new _syscall_verify_* function exists for every syscall, which
calls the real implementation. The syscall table for the syscall handler
now maps to these verify functions.

Other changes:

- Updated the definition grammar to allow options to have a "key:value"
  style, to eventually support capabilities.
- Added an "optional" option for parameters that says a syscall will
  accept a null value.
- Some bonnibel fixes, as definition file changes weren't always
  properly causing updates in the build dep graph.
- The syscall implementation function signatures are no longer exposed
  in syscall.h. Also, the unused syscall enum has been removed.

assets/grammars/definitions.g

@@ -26,13 +26,14 @@ object_name: "object" name
 
 id: NUMBER
 name: IDENTIFIER
-options: "[" IDENTIFIER+ "]"
+options: "[" ( OPTION | IDENTIFIER )+ "]"
 description: COMMENT+
 
 PRIMITIVE: INT_TYPE | "size" | "string" | "buffer" | "address"
 INT_TYPE: /u?int(8|16|32|64)?/
 NUMBER: /(0x)?[0-9a-fA-F]+/
 UID: /[0-9a-fA-F]{16}/
+OPTION.2: IDENTIFIER ":" IDENTIFIER
 COMMENT: /#.*/
 PATH: /"[^"]*"/
 

definitions/objects/endpoint.def

@@ -17,7 +17,7 @@ object endpoint : kobject {
     # is available.
     method receive {
         param tag uint64 [out]
-        param data buffer [out]
+        param data buffer [out optional]
         param timeout uint64    # Receive timeout in nanoseconds
     }
 

definitions/objects/process.def

@@ -28,7 +28,7 @@ object process : kobject {
     # Give the given process a handle that points to the same
     # object as the specified handle.
     method give_handle {
-        param sender object kobject          # A handle in the caller process to send
+        param sender object kobject                   # A handle in the caller process to send
-        param receiver object kobject [out]  # The handle as the recipient will see it
+        param receiver object kobject [out optional]  # The handle as the recipient will see it
     }
 }

definitions/objects/system.def

@@ -8,7 +8,7 @@ object system : kobject {
 
     # Get a log line from the kernel log
     method get_log {
-        param buffer buffer [out]   # Buffer for the log message data structure
+        param buffer buffer [out optional]   # Buffer for the log message data structure
     }
 
     # Ask the kernel to send this process messages whenever

requirements.txt

@@ -2,3 +2,4 @@ cogapp >= 3
 ninja >= 1.10.2
 peru >= 1.2.1
 pyyaml >= 5.4
+lark == 0.12.0

scripts/bonnibel/module.py

@@ -141,6 +141,7 @@ class Module:
 
             inputs = []
             parse_deps = []
+            parse_dep = "${module_dir}/.parse_dep.phony"
 
             for start in self.sources:
                 source = start
@@ -148,6 +149,11 @@ class Module:
                 while source and source.action:
                     output = source.output
 
+                    if source.action.parse_deps:
+                        oodeps = [parse_dep]
+                    else:
+                        oodeps = []
+
                     if source.action.rule:
                         build.newline()
                         build.build(
@@ -156,6 +162,7 @@ class Module:
                             inputs = source.input,
                             implicit = list(map(resolve, source.deps)) +
                                        list(source.action.deps),
+                            order_only = oodeps,
                             variables = {"name": source.name},
                         )
 
@@ -167,7 +174,6 @@ class Module:
 
                     source = output
 
-            parse_dep = "${module_dir}/.parse_dep.phony"
             build.newline()
             build.build(
                 rule = "touch",
@@ -183,7 +189,7 @@ class Module:
                 rule = self.kind,
                 outputs = output,
                 inputs = inputs,
-                implicit = [parse_dep] + libs,
+                implicit = libs,
                 order_only = order_only,
             )
 

scripts/bonnibel/source.py

@@ -3,6 +3,7 @@ class Action:
     implicit = property(lambda self: False)
     rule = property(lambda self: None)
     deps = property(lambda self: tuple())
+    parse_deps = property(lambda self: False)
 
     def __init__(self, name):
         self.__name = name
@@ -14,6 +15,7 @@ class Action:
 class Compile(Action):
     rule = property(lambda self: f'compile_{self.name}')
     deps = property(lambda self: ("${module_dir}/.parse_dep.phony",))
+    parse_deps = property(lambda self: True)
 
     def __init__(self, name, suffix = ".o"):
         super().__init__(name)

scripts/definitions/context.py

@@ -34,6 +34,7 @@ class Context:
         pending = set()
         pending.add(filename)
 
+        from .parser import LarkError
         from .parser import Lark_StandAlone as Parser
         from .transformer import DefTransformer
 
@@ -47,7 +48,16 @@ class Context:
             path = self.find(name)
 
             parser = Parser(transformer=DefTransformer(name))
-            imps, objs, ints = parser.parse(open(path, "r").read())
+
+            try:
+                imps, objs, ints = parser.parse(open(path, "r").read())
+            except LarkError as e:
+                import sys
+                import textwrap
+                print(f"\nError parsing {name}:", file=sys.stderr)
+                print(textwrap.indent(str(e), "    "), file=sys.stderr)
+                sys.exit(1)
+
             objects.update(objs)
             interfaces.update(ints)
 

scripts/definitions/types/__init__.py

@@ -5,10 +5,15 @@ def _indent(x):
 class Description(str): pass
 class Import(str): pass
 
-class Options(set):
+class Options(dict):
+    def __init__(self, opts = tuple()):
+        for opt in opts:
+            parts = opt.split(":", 1)
+            self[parts[0]] = "".join(parts[1:])
+
     def __str__(self):
         if not self: return ""
-        return "[{}]".format(" ".join(self))
+        return "[{}]".format(" ".join(self.keys()))
 
 class UID(int):
     def __str__(self):

scripts/definitions/types/function.py

@@ -47,3 +47,15 @@ class Param:
         return "param {} {} {}  {}".format(
                 self.name, repr(self.type), self.options, self.desc or "")
 
+    @property
+    def outparam(self):
+        return "out" in self.options or "inout" in self.options
+
+    @property
+    def refparam(self):
+        return self.type.reference or self.outparam
+
+    @property
+    def optional(self):
+        return "optional" in self.options
+

scripts/definitions/types/primitive.py

@@ -8,9 +8,9 @@ class Primitive(Type):
     def __repr__(self):
         return f'Primitive({self.name})'
 
-    def c_names(self, options=None):
+    def c_names(self, options=dict()):
         one = self.c_type
-        if options and "out" in options or "inout" in options:
+        if "out" in options or "inout" in options:
             one += " *"
 
         return ((one, ""),)
@@ -23,11 +23,13 @@ class PrimitiveRef(Primitive):
         super().__init__(name, c_type)
         self.__counted = counted
 
-    def c_names(self, options=None):
+    reference = property(lambda self: True)
+
+    def c_names(self, options=dict()):
         one = f"{self.c_type} *"
         two = "size_t"
 
-        if options and "out" in options or "inout" in options:
+        if "out" in options or "inout" in options:
             two += " *"
         else:
             one = "const " + one

scripts/definitions/types/type.py

@@ -3,6 +3,7 @@ class Type:
         self.__name = name
 
     name = property(lambda self: self.__name)
+    reference = property(lambda self: False)
 
     def c_names(self, options):
         raise NotImplemented("Call to base Type.c_names")

src/kernel/kernel.module

@@ -48,6 +48,7 @@ kernel = module("kernel",
         "syscall.cpp.cog",
         "syscall.h.cog",
         "syscall.s",
+        "syscall_verify.cpp.cog",
         "syscalls.inc.cog",
         "syscalls/channel.cpp",
         "syscalls/endpoint.cpp",
@@ -68,10 +69,15 @@ from os.path import join
 
 layout = join(source_root, "definitions/memory_layout.yaml")
 sysconf = join(source_root, "definitions/sysconf.yaml")
-definitions = glob('definitions/**/*.def', recursive=True)
+definitions = glob(join(source_root, 'definitions/**/*.def'), recursive=True)
 
 kernel.add_depends(["memory.h.cog"], [layout])
 kernel.add_depends(["sysconf.h.cog"], [sysconf])
-kernel.add_depends(["syscall.cpp.cog", "syscall.h.cog", "syscalls.inc.cog"], definitions)
+kernel.add_depends([
+        "syscall.cpp.cog",
+        "syscall.h.cog",
+        "syscalls.inc.cog",
+        "syscall_verify.cpp.cog",
+    ], definitions)
 
 kernel.variables['ldflags'] = ["${ldflags}", "-T", "${source_root}/src/kernel/kernel.ld"]

src/kernel/syscall.cpp.cog

@@ -21,9 +21,39 @@ ctx = Context(definitions_path)
 ctx.parse("syscalls.def")
 syscalls = ctx.interfaces['syscalls']
 
-cog.outl(f"constexpr size_t num_syscalls = {len(syscalls.methods)};")
 ]]]*/
 /// [[[end]]]
+
+namespace syscalls
+{
+    /*[[[cog code generation
+
+    last_scope = None
+    for id, scope, method in syscalls.methods:
+        if scope != last_scope:
+            cog.outl()
+            last_scope = scope
+
+        if scope:
+            name = f"{scope.name}_{method.name}"
+        else:
+            name = method.name
+
+        args = []
+        if method.constructor:
+            args.append("j6_handle_t *handle")
+        elif not method.static:
+            args.append("j6_handle_t handle")
+
+        for param in method.params:
+            for type, suffix in param.type.c_names(param.options):
+                args.append(f"{type} {param.name}{suffix}")
+
+        cog.outl(f"""j6_status_t _syscall_verify_{name} ({", ".join(args)});""")
+    ]]]*/
+    //[[[end]]]
+}
+
 uintptr_t syscall_registry[num_syscalls] __attribute__((section(".syscall_registry")));
 
 void
@@ -44,8 +74,8 @@ syscall_initialize()
         else:
             name = method.name
 
-        cog.outl(f"syscall_registry[{id}] = reinterpret_cast<uintptr_t>(syscalls::{name});")
+        cog.outl(f"syscall_registry[{id}] = reinterpret_cast<uintptr_t>(syscalls::_syscall_verify_{name});")
-        cog.outl(f"""log::debug(logs::syscall, "Enabling syscall {id:x} as {name}");""")
+        cog.outl(f"""log::debug(logs::syscall, "Enabling syscall {id:02x} as {name}");""")
         cog.outl("")
     ]]]*/
     //[[[end]]]

src/kernel/syscall.h.cog

@@ -13,48 +13,9 @@ ctx = Context(definitions_path)
 ctx.parse("syscalls.def")
 syscalls = ctx.interfaces["syscalls"]
 
+cog.outl(f"constexpr size_t num_syscalls = {len(syscalls.methods)};")
 ]]]*/
 /// [[[end]]]
 
-enum class syscall : uint64_t
-{
-    /*[[[cog code generation
-
-    for id, scope, method in syscalls.methods:
-        if scope:
-            name = f"{scope.name}_{method.name}"
-        else:
-            name = method.name
-        cog.outl(f"{name:20} = {id},")
-
-    ]]]*/
-    //[[[end]]]
-};
-
 void syscall_initialize();
 extern "C" void syscall_enable();
-
-namespace syscalls
-{
-/*[[[cog code generation
-
-for id, scope, method in syscalls.methods:
-    if scope:
-        name = f"{scope.name}_{method.name}"
-    else:
-        name = method.name
-
-    args = []
-    if method.constructor:
-        args.append("j6_handle_t *handle")
-    elif not method.static:
-        args.append("j6_handle_t handle")
-
-    for param in method.params:
-        for type, suffix in param.type.c_names(param.options):
-            args.append(f"{type} {param.name}{suffix}")
-
-    cog.outl(f"""j6_status_t {name} ({", ".join(args)});""")
-]]]*/
-//[[[end]]]
-}

src/kernel/syscall_verify.cpp.cog

@@ -0,0 +1,73 @@
+// vim: ft=cpp
+
+#include <stdint.h>
+#include <arch/memory.h>
+#include <j6/errors.h>
+#include <j6/types.h>
+
+namespace {
+    template <typename T>
+    __attribute__((always_inline))
+    inline bool check_refparam(const T* param, bool optional) {
+        return reinterpret_cast<uintptr_t>(param) < arch::kernel_offset &&
+            (optional || param);
+    }
+}
+
+namespace syscalls {
+
+/*[[[cog code generation
+from definitions.context import Context
+
+ctx = Context(definitions_path)
+ctx.parse("syscalls.def")
+syscalls = ctx.interfaces["syscalls"]
+
+cbool = {True: "true", False: "false"}
+
+for id, scope, method in syscalls.methods:
+    if scope:
+        name = f"{scope.name}_{method.name}"
+    else:
+        name = f"{method.name}"
+
+    args = []
+    argdefs = []
+    refparams = []
+
+    if method.constructor:
+        argdefs.append("j6_handle_t *handle")
+        args.append("handle")
+        refparams.append(("handle", False))
+    elif not method.static:
+        argdefs.append("j6_handle_t handle")
+        args.append("handle")
+
+    for param in method.params:
+        for type, suffix in param.type.c_names(param.options):
+            arg = f"{param.name}{suffix}"
+            argdefs.append(f"{type} {arg}")
+            args.append(arg)
+
+        if param.refparam:
+            subs = param.type.c_names(param.options)
+            refparams.append((param.name + subs[0][1], param.optional))
+            if param.outparam:
+                for sub in subs[1:]:
+                    refparams.append((param.name + sub[1], param.optional))
+
+    cog.outl(f"""extern j6_status_t {name} ({", ".join(argdefs)});""")
+    cog.outl(f"""j6_status_t _syscall_verify_{name} ({", ".join(argdefs)}) {{""")
+
+    for pname, optional in refparams:
+        cog.outl(f"    if (!check_refparam({pname}, {cbool[optional]}))")
+        cog.outl( "        return j6_err_invalid_arg;")
+        cog.outl()
+
+    cog.outl(f"""    return syscalls::{name}({", ".join(args)});""")
+    cog.outl("}")
+    cog.outl("\n")
+]]]*/
+//[[[end]]]
+
+} // namespace syscalls

src/kernel/syscalls/endpoint.cpp

@@ -29,7 +29,9 @@ endpoint_send(j6_handle_t handle, uint64_t tag, const void * data, size_t data_l
 j6_status_t
 endpoint_receive(j6_handle_t handle, uint64_t * tag, void * data, size_t * data_len, uint64_t timeout)
 {
-    if (!tag || !data_len || (*data_len && !data))
+    // Data is marked optional, but we need the length, and if length > 0,
+    // data is not optional.
+    if (!data_len || (*data_len && !data))
         return j6_err_invalid_arg;
 
     endpoint *e = get_handle<endpoint>(handle);
@@ -46,7 +48,7 @@ endpoint_receive(j6_handle_t handle, uint64_t * tag, void * data, size_t * data_
 j6_status_t
 endpoint_sendrecv(j6_handle_t handle, uint64_t * tag, void * data, size_t * data_len, uint64_t timeout)
 {
-    if (!tag || (*tag & j6_tag_system_flag))
+    if (*tag & j6_tag_system_flag)
         return j6_err_invalid_arg;
 
     endpoint *e = get_handle<endpoint>(handle);

src/kernel/syscalls/object.cpp

@@ -13,9 +13,6 @@ namespace syscalls {
 j6_status_t
 kobject_koid(j6_handle_t handle, j6_koid_t *koid)
 {
-    if (koid == nullptr)
-        return j6_err_invalid_arg;
-
     kobject *obj = get_handle<kobject>(handle);
     if (!obj)
         return j6_err_invalid_arg;

src/kernel/syscalls/system.cpp

@@ -19,9 +19,6 @@ namespace syscalls {
 j6_status_t
 log(const char *message)
 {
-    if (message == nullptr)
-        return j6_err_invalid_arg;
-
     thread &th = thread::current();
     log::info(logs::syscall, "Message[%llx]: %s", th.koid(), message);
     return j6_status_ok;
@@ -36,23 +33,24 @@ noop()
 }
 
 j6_status_t
-system_get_log(j6_handle_t sys, void *buffer, size_t *size)
+system_get_log(j6_handle_t sys, void *buffer, size_t *buffer_len)
 {
-    if (!size || (*size && !buffer))
+    // Buffer is marked optional, but we need the length, and if length > 0,
+    // buffer is not optional.
+    if (!buffer_len || (*buffer_len && !buffer))
         return j6_err_invalid_arg;
 
-    size_t orig_size = *size;
+    size_t orig_size = *buffer_len;
-    *size = g_logger.get_entry(buffer, *size);
+    *buffer_len = g_logger.get_entry(buffer, *buffer_len);
     if (!g_logger.has_log())
         system::get().deassert_signal(j6_signal_system_has_log);
 
-    return (*size > orig_size) ? j6_err_insufficient : j6_status_ok;
+    return (*buffer_len > orig_size) ? j6_err_insufficient : j6_status_ok;
 }
 
 j6_status_t
 system_bind_irq(j6_handle_t sys, j6_handle_t endp, unsigned irq)
 {
-    // TODO: check capabilities on sys handle
     endpoint *e = get_handle<endpoint>(endp);
     if (!e) return j6_err_invalid_arg;
 
@@ -65,9 +63,6 @@ system_bind_irq(j6_handle_t sys, j6_handle_t endp, unsigned irq)
 j6_status_t
 system_map_phys(j6_handle_t handle, j6_handle_t * area, uintptr_t phys, size_t size, uint32_t flags)
 {
-    // TODO: check capabilities on sys handle
-    if (!area) return j6_err_invalid_arg;
-
     // TODO: check to see if frames are already used? How would that collide with
     // the bootloader's allocated pages already being marked used?
     if (!(flags & vm_flags::mmio))
@@ -82,7 +77,6 @@ system_map_phys(j6_handle_t handle, j6_handle_t * area, uintptr_t phys, size_t s
 j6_status_t
 system_request_iopl(j6_handle_t handle, unsigned iopl)
 {
-    // TODO: check capabilities on sys handle
     if (iopl != 0 && iopl != 3)
         return j6_err_invalid_arg;
 

src/kernel/syscalls/vm_area.cpp

@@ -56,9 +56,6 @@ vma_unmap(j6_handle_t handle, j6_handle_t proc)
 j6_status_t
 vma_resize(j6_handle_t handle, size_t *size)
 {
-    if (!size)
-        return j6_err_invalid_arg;
-
     vm_area *a = get_handle<vm_area>(handle);
     if (!a) return j6_err_invalid_arg;