From: Arran Cudbard-Bell Date: Mon, 13 May 2024 00:06:05 +0000 (-0600) Subject: Fixes to get listeners to work with mprotect X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=feb591250857a80042dfb99e7afffa4939856fc4;p=thirdparty%2Ffreeradius-server.git Fixes to get listeners to work with mprotect --- diff --git a/src/lib/io/master.c b/src/lib/io/master.c index 42d6cc30c91..3a090be5cdb 100644 --- a/src/lib/io/master.c +++ b/src/lib/io/master.c @@ -2908,7 +2908,7 @@ int fr_io_listen_free(fr_listen_t *li) return 0; } -int fr_master_io_listen(TALLOC_CTX *ctx, fr_io_instance_t *inst, fr_schedule_t *sc, +int fr_master_io_listen(fr_io_instance_t *inst, fr_schedule_t *sc, size_t default_message_size, size_t num_messages) { fr_listen_t *li, *child; diff --git a/src/lib/io/master.h b/src/lib/io/master.h index fef64cb3c91..dcd57485c42 100644 --- a/src/lib/io/master.h +++ b/src/lib/io/master.h @@ -105,7 +105,7 @@ typedef struct { extern fr_app_io_t fr_master_app_io; fr_trie_t *fr_master_io_network(TALLOC_CTX *ctx, int af, fr_ipaddr_t *allow, fr_ipaddr_t *deny); -int fr_master_io_listen(TALLOC_CTX *ctx, fr_io_instance_t *io, fr_schedule_t *sc, +int fr_master_io_listen(fr_io_instance_t *io, fr_schedule_t *sc, size_t default_message_size, size_t num_messages) CC_HINT(nonnull); fr_io_track_t *fr_master_io_track_alloc(fr_listen_t *li, fr_client_t *client, fr_ipaddr_t const *src_ipaddr, int src_port, fr_ipaddr_t const *dst_ipaddr, int dst_port); diff --git a/src/lib/server/module.c b/src/lib/server/module.c index 8119f647f3e..3c4a30f514d 100644 --- a/src/lib/server/module.c +++ b/src/lib/server/module.c @@ -638,12 +638,12 @@ int module_data_protect(module_instance_t *mi, module_data_pool_t *pool) if (pool->start == NULL) return 0; /* noop */ DEBUG("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; } @@ -655,20 +655,44 @@ int module_data_protect(module_instance_t *mi, module_data_pool_t *pool) * - -1 on failure. */ static inline CC_HINT(always_inline) -int module_data_unprotect(module_instance_t *mi, module_data_pool_t *pool) +int module_data_unprotect(module_instance_t const *mi, module_data_pool_t const *pool) { if (pool->start == NULL) return 0; /* noop */ DEBUG("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; } +/** Mark module data as read only + * + * @param[in] mi Instance data to protect (mark as read only). + * @return + * - 0 on success. + * - -1 on failure. + */ +int module_instance_data_protect(module_instance_t const *mi) +{ + return module_data_unprotect(mi, &mi->inst_pool); +} + +/** Mark module data as read/write + * + * @param[in] mi Instance data to unprotect (mark as read/write). + * @return + * - 0 on success. + * - -1 on failure. + */ +int module_instance_data_unprotect(module_instance_t const *mi) +{ + return module_data_unprotect(mi, &mi->inst_pool); +} + /** Return the prefix string for the deepest module * * This is useful for submodules which don't have a prefix of their own. diff --git a/src/lib/server/module.h b/src/lib/server/module.h index 99e69c96be2..41d9ec42bb2 100644 --- a/src/lib/server/module.h +++ b/src/lib/server/module.h @@ -388,6 +388,19 @@ void module_instance_debug(module_instance_t const *mi) CC_HINT(nonnull); void module_list_debug(module_list_t const *ml) CC_HINT(nonnull); /** @} */ +/** @name Toggle protection on module instance data + * + * This is used for module lists which implement additional instantiation phases + * (like li->open). It should NOT be used by modules to hack around instance + * data being read-only after instantiation completes. + * + * @{ + */ +int module_instance_data_protect(module_instance_t const *mi); + +int module_instance_data_unprotect(module_instance_t const *mi); + /** @} */ + /** @name Module and module thread lookup * * @{ diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c index de919617b33..c63cc65c6bd 100644 --- a/src/lib/server/virtual_servers.c +++ b/src/lib/server/virtual_servers.c @@ -1392,13 +1392,25 @@ int virtual_servers_open(fr_schedule_t *sc) * Even then, proto_radius usually calls fr_master_io_listen() in order * to create the fr_listen_t structure. */ - if (listener->proto_module->open && - listener->proto_module->open(listener->proto_mi->data, sc, - listener->proto_mi->conf) < 0) { - cf_log_err(listener->proto_mi->conf, - "Opening %s I/O interface failed", - listener->proto_module->common.name); - return -1; + if (listener->proto_module->open) { + int ret; + + /* + * Sometimes the open function needs to modify instance + * data, so we need to temporarily remove the protection. + */ + module_instance_data_unprotect(listener->proto_mi); + ret = listener->proto_module->open(listener->proto_mi->data, sc, + listener->proto_mi->conf); + module_instance_data_protect(listener->proto_mi); + if (unlikely(ret < 0)) { + cf_log_err(listener->proto_mi->conf, + "Opening %s I/O interface failed", + listener->proto_module->common.name); + + return -1; + } + } /* diff --git a/src/listen/bfd/proto_bfd.c b/src/listen/bfd/proto_bfd.c index e4e609d1718..0a38eead00a 100644 --- a/src/listen/bfd/proto_bfd.c +++ b/src/listen/bfd/proto_bfd.c @@ -303,7 +303,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf /* * io.app_io should already be set */ - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/control/proto_control.c b/src/listen/control/proto_control.c index 25816f62839..4a9019c0e69 100644 --- a/src/listen/control/proto_control.c +++ b/src/listen/control/proto_control.c @@ -80,7 +80,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf inst->io.app = &proto_control; inst->io.app_instance = instance; - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/cron/proto_cron.c b/src/listen/cron/proto_cron.c index 05f5c7c9490..7b911c831f8 100644 --- a/src/listen/cron/proto_cron.c +++ b/src/listen/cron/proto_cron.c @@ -161,7 +161,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf /* * io.app_io should already be set */ - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/detail/proto_detail.c b/src/listen/detail/proto_detail.c index 0b205159b5a..cbef9ff4eaf 100644 --- a/src/listen/detail/proto_detail.c +++ b/src/listen/detail/proto_detail.c @@ -436,11 +436,11 @@ static int mod_open(void *instance, fr_schedule_t *sc, CONF_SECTION *conf) * path, data takes from the socket to the decoder and * back again. */ - li = talloc_zero(inst, fr_listen_t); + MEM(li = talloc_zero(NULL, fr_listen_t)); /* Assigned thread steals the memory */ talloc_set_destructor(li, fr_io_listen_free); li->app_io = inst->app_io; - li->thread_instance = talloc_zero_array(NULL, uint8_t, li->app_io->common.thread_inst_size); + li->thread_instance = talloc_zero_array(li, uint8_t, li->app_io->common.thread_inst_size); talloc_set_name(li->thread_instance, "proto_%s_thread_t", inst->app_io->common.name); li->app_io_instance = inst->app_io_instance; diff --git a/src/listen/dhcpv4/proto_dhcpv4.c b/src/listen/dhcpv4/proto_dhcpv4.c index 10e9900af76..0d425f3ec0d 100644 --- a/src/listen/dhcpv4/proto_dhcpv4.c +++ b/src/listen/dhcpv4/proto_dhcpv4.c @@ -349,7 +349,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf inst->io.app = &proto_dhcpv4; inst->io.app_instance = instance; - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/dhcpv6/proto_dhcpv6.c b/src/listen/dhcpv6/proto_dhcpv6.c index e000980eb9b..172e226f423 100644 --- a/src/listen/dhcpv6/proto_dhcpv6.c +++ b/src/listen/dhcpv6/proto_dhcpv6.c @@ -330,7 +330,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf inst->io.app = &proto_dhcpv6; inst->io.app_instance = instance; - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/dns/proto_dns.c b/src/listen/dns/proto_dns.c index 00885468edb..b589c32b65e 100644 --- a/src/listen/dns/proto_dns.c +++ b/src/listen/dns/proto_dns.c @@ -279,7 +279,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf inst->io.app = &proto_dns; inst->io.app_instance = instance; - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/load/proto_load.c b/src/listen/load/proto_load.c index fc43f3bf1e1..386053157aa 100644 --- a/src/listen/load/proto_load.c +++ b/src/listen/load/proto_load.c @@ -171,7 +171,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf /* * io.app_io should already be set */ - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/radius/proto_radius.c b/src/listen/radius/proto_radius.c index 488f1f362cf..e582281d94e 100644 --- a/src/listen/radius/proto_radius.c +++ b/src/listen/radius/proto_radius.c @@ -418,7 +418,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf /* * io.app_io should already be set */ - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } diff --git a/src/listen/radius/proto_radius_udp.c b/src/listen/radius/proto_radius_udp.c index a7f948ba9a9..ea9cc9e7c0a 100644 --- a/src/listen/radius/proto_radius_udp.c +++ b/src/listen/radius/proto_radius_udp.c @@ -176,7 +176,7 @@ static ssize_t mod_read(fr_listen_t *li, void **packet_ctx, fr_time_t *recv_time /* * @todo - check for F5 load balancer packets. */ - DEBUG2("proto_radius_udp got a packet which isn't RADIUS"); + DEBUG2("proto_radius_udp got a packet which isn't RADIUS: %s", fr_strerror()); thread->stats.total_malformed_requests++; return 0; } diff --git a/src/listen/tacacs/proto_tacacs.c b/src/listen/tacacs/proto_tacacs.c index bec40e3e917..b2eea3208ea 100644 --- a/src/listen/tacacs/proto_tacacs.c +++ b/src/listen/tacacs/proto_tacacs.c @@ -437,7 +437,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf /* * io.app_io should already be set */ - return fr_master_io_listen(inst, &inst->io, sc, inst->max_packet_size, inst->num_messages); + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); } /** Instantiate the application diff --git a/src/listen/vmps/proto_vmps.c b/src/listen/vmps/proto_vmps.c index 086e16264fa..c57663fad7f 100644 --- a/src/listen/vmps/proto_vmps.c +++ b/src/listen/vmps/proto_vmps.c @@ -298,7 +298,7 @@ static int mod_open(void *instance, fr_schedule_t *sc, UNUSED CONF_SECTION *conf inst->io.app = &proto_vmps; inst->io.app_instance = instance; - return fr_master_io_listen(inst, &inst->io, sc, + return fr_master_io_listen(&inst->io, sc, inst->max_packet_size, inst->num_messages); }