Enable write protection on a per-module list basis

diff --git a/src/lib/io/master.c b/src/lib/io/master.c
index e89506578ec11fb01ea13b15273c74d46ef44239..87729313347be80dc11306df97ce10f66ca3c0d8 100644 (file)
--- a/src/lib/io/master.c
+++ b/src/lib/io/master.c
@@ -2702,8 +2702,10 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
         *      Create a list of client modules.
         *
         *      FIXME - Probably only want to do this for connected sockets?
+        *
+        *      FIXME - We probably want write protect enabled?
         */
-       inst->clients = module_list_alloc(inst, &module_list_type_thread_local, "clients");
+       inst->clients = module_list_alloc(inst, &module_list_type_thread_local, "clients", false);
        module_list_mask_set(inst->clients, MODULE_INSTANCE_BOOTSTRAPPED);
 
        return 0;

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index c073e78b106365f521159623a7f045349c73550d..6e036ee07369444f2c6361c64aff67470ceeefbf 100644 (file)
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -622,16 +622,14 @@ void module_list_debug(module_list_t const *ml)
 static inline CC_HINT(always_inline)
 int module_data_protect(module_instance_t *mi, module_data_pool_t *pool)
 {
-       if (pool->start == NULL) return 0; /* noop */
+       if ((pool->start == NULL) || !mi->ml->write_protect) return 0; /* noop */
 
        DEBUG3("Protecting data %s %p-%p", mi->name, pool->start, (uint8_t *)pool->start + pool->len);
 
-#if 0
        if (unlikely(mprotect(pool->start, pool->len, PROT_READ) < 0)) {
                fr_strerror_printf("Protecting \"%s\" module data failed: %s", mi->name, fr_syserror(errno));
                return -1;
        }
-#endif
 
        return 0;
 }
@@ -646,16 +644,14 @@ int module_data_protect(module_instance_t *mi, module_data_pool_t *pool)
 static inline CC_HINT(always_inline)
 int module_data_unprotect(module_instance_t const *mi, module_data_pool_t const *pool)
 {
-       if (pool->start == NULL) return 0; /* noop */
+       if ((pool->start == NULL) || !mi->ml->write_protect) return 0; /* noop */
 
        DEBUG3("Unprotecting data %s %p-%p", mi->name, pool->start, (uint8_t *)pool->start + pool->len);
 
-#if 0
        if (unlikely(mprotect(pool->start, pool->len, PROT_READ | PROT_WRITE) < 0)) {
                fr_strerror_printf("Unprotecting \"%s\" data failed: %s", mi->name, fr_syserror(errno));
                return -1;
        }
-#endif
 
        return 0;
 }
@@ -1747,14 +1743,17 @@ void module_list_mask_set(module_list_t *ml, module_instance_state_t mask)
  * If the list is freed all module instance data will be freed.
  * If no more instances of the module exist the module be unloaded.
  *
- * @param[in] ctx      To allocate the list in.
- * @param[in] type     of the list.  Controls whether this is a global
- *                     module list, or a per-thread list containing
- *                     variants of an existing module.
- * @param[in] name     of the list.  Used for debugging.
+ * @param[in] ctx              To allocate the list in.
+ * @param[in] type             of the list.  Controls whether this is a global
+ *                             module list, or a per-thread list containing
+ *                             variants of an existing module.
+ * @param[in] name             of the list.  Used for debugging.
+ * @param[in] write_protect    Whether to write protect the module data
+ *                             after instantiation and bootstrapping.
  * @return A new module list.
  */
-module_list_t *module_list_alloc(TALLOC_CTX *ctx, module_list_type_t const *type, char const *name)
+module_list_t *module_list_alloc(TALLOC_CTX *ctx, module_list_type_t const *type,
+                                char const *name, bool write_protect)
 {
        module_list_t *ml;
 
@@ -1778,6 +1777,7 @@ module_list_t *module_list_alloc(TALLOC_CTX *ctx, module_list_type_t const *type
                talloc_free(ml);
                return NULL;
        }
+       ml->write_protect = write_protect;
 
        return ml;
 }

diff --git a/src/lib/server/module.h b/src/lib/server/module.h
index 41d9ec42bb2e78e4b47939e6bd5bfcb504adaced..ed05320fdd6abe6adfdf5cfcc348cfe192790dae 100644 (file)
--- a/src/lib/server/module.h
+++ b/src/lib/server/module.h
@@ -331,8 +331,13 @@ typedef module_thread_instance_t *(*module_list_thread_data_get_t)(module_instan
 
 /** A list of modules
  *
- * This allows modules to be instantiated and freed in phases,
- * i.e. proto modules before rlm modules.
+ * This used to be a global structure, but was move to a struct.
+ *
+ * Module lists allow collections of modules to be created.  The module lists themselves can be configured
+ * to be thread-local or global, with optional runtime write protection.
+ *
+ * Thread-local module lists are used for dynamic modules, i.e. those created at runtime, where as the
+ * global module lists are used for backend modules, listeners, and process state machines.
  */
 struct module_list_s
 {
@@ -344,6 +349,11 @@ struct module_list_s
        fr_rb_tree_t                    *data_tree;             //!< Modules indexed by data.
        fr_heap_t                       *inst_heap;             //!< Heap of module instances.
 
+       bool                            write_protect;          //!< If true, pages containing module boot or
+                                                               ///< instance data will be write protected after
+                                                               ///< bootstrapping and instantiation is complete,
+                                                               ///< to prevent accidental modification.
+
        /** @name Callbacks to manage thread-specific data
         *
         * In "child" lists, which are only operating in a single thread, we don't need
@@ -491,7 +501,8 @@ bool                        module_instance_skip_thread_instantiate(module_instance_t *mi);
 void                   module_list_mask_set(module_list_t *ml, module_instance_state_t mask);
 /** @} */
 
-module_list_t          *module_list_alloc(TALLOC_CTX *ctx, module_list_type_t const *type, char const *name)
+module_list_t          *module_list_alloc(TALLOC_CTX *ctx, module_list_type_t const *type,
+                                          char const *name, bool write_protect)
                                           CC_HINT(nonnull(2,3)) CC_HINT(warn_unused_result);
 
 void                   modules_init(char const *lib_dir);

diff --git a/src/lib/server/module_rlm.c b/src/lib/server/module_rlm.c
index a5658c93569e73089d33439a28cd1513f766fe1f..ee13d73a148507c656663ac641d3b58d1626d342 100644 (file)
--- a/src/lib/server/module_rlm.c
+++ b/src/lib/server/module_rlm.c
@@ -1170,7 +1170,7 @@ static int _modules_rlm_free_atexit(UNUSED void *uctx)
  */
 int modules_rlm_init(void)
 {
-       MEM(rlm_modules = module_list_alloc(NULL, &module_list_type_global, "rlm"));
+       MEM(rlm_modules = module_list_alloc(NULL, &module_list_type_global, "rlm", true));
        MEM(module_rlm_virtual_name_tree = fr_rb_inline_alloc(NULL, module_rlm_virtual_t, name_node,
                                                              module_rlm_virtual_name_cmp, NULL));
        fr_atexit_global(_modules_rlm_free_atexit, NULL);

diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c
index c63cc65c6bd88214e768501eb839f64ae487d015..f3f6e9c3a9aa54cf3f50066534e063238f452515 100644 (file)
--- a/src/lib/server/virtual_servers.c
+++ b/src/lib/server/virtual_servers.c
@@ -1626,8 +1626,14 @@ int virtual_servers_init(void)
                return -1;
        }
 
-       MEM(process_modules = module_list_alloc(NULL, &module_list_type_global, "process"));
-       MEM(proto_modules = module_list_alloc(NULL, &module_list_type_global, "protocol"));
+       MEM(process_modules = module_list_alloc(NULL, &module_list_type_global, "process", true));
+
+       /*
+        *      FIXME - We should be able to turn on write protection,
+        *      but there are too many proto modules that hang things
+        *      off of their instance data.
+        */
+       MEM(proto_modules = module_list_alloc(NULL, &module_list_type_global, "protocol", false));
        MEM(listen_addr_root = fr_rb_inline_alloc(NULL, fr_listen_t, virtual_server_node, listen_addr_cmp, NULL));
        MEM(server_section_name_tree = fr_rb_alloc(NULL, server_section_name_cmp, NULL));