diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index 50ebb3591fa..643d63bccf7 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -129,12 +129,15 @@ int open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev); void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process DCO messages from the kernel. + * + * Each message is processed immediately inside the netlink callback + * to prevent message loss when multiple notifications arrive simultaneously. * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -301,7 +304,7 @@ close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx) } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 13a08319a24..ec7fee24644 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -559,7 +559,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid, } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index b2584b973ce..88f836f339e 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -59,10 +59,20 @@ nla_nest_start(_msg, (_type) | NLA_F_NESTED) static int ovpn_get_mcast_id(dco_context_t *dco); +static int ovpn_handle_msg(struct nl_msg *msg, void *arg); +static int mcast_family_handler(struct nl_msg *msg, void *arg); +static int dco_parse_peer_multi(struct nl_msg *msg, void *arg); +static int dco_parse_peer(struct nl_msg *msg, void *arg); + +/* Forward declarations for immediate processing in callback */ +void multi_process_incoming_dco(dco_context_t *dco); +void process_incoming_dco(dco_context_t *dco); void dco_check_key_ctx(const struct key_ctx_bi *key); -typedef int (*ovpn_nl_cb)(struct nl_msg *msg, void *arg); +/* Lock to prevent recursive stats requests during message processing. + * When set, dco_get_peer_stats() returns early to avoid NLE_BUSY errors. */ +static bool __is_locked = false; /** * @brief resolves the netlink ID for ovpn-dco @@ -131,7 +141,9 @@ ovpn_dco_nlmsg_create(dco_context_t *dco, enum ovpn_nl_commands cmd) static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { + __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); + __is_locked = false; switch (ret) { @@ -167,23 +179,22 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) } /** - * Send a prepared netlink message and registers cb as callback if non-null. + * Send a prepared netlink message. + * + * The callback is permanently registered in ovpn_dco_init_netlink() and routes + * messages to appropriate handlers based on type. * - * The method will also free nl_msg * @param dco The dco context to use * @param nl_msg the message to use - * @param cb An optional callback if the caller expects an answer - * @param cb_arg An optional param to pass to the callback * @param prefix A prefix to report in the error message to give the user context * @return status of sending the message */ static int -ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, ovpn_nl_cb cb, - void *cb_arg, const char *prefix) +ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, const char *prefix) { dco->status = 1; - nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, cb_arg); + /* Callback permanently set in ovpn_dco_init_netlink() - don't register here */ nl_send_auto(dco->nl_sock, nl_msg); while (dco->status == 1) @@ -274,7 +285,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, } nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -379,6 +390,14 @@ ovpn_dco_init_netlink(dco_context_t *dco) nl_cb_set(dco->nl_cb, NL_CB_ACK, NL_CB_CUSTOM, ovpn_nl_cb_finish, &dco->status); + /* Store NLCTRL family ID for message routing in dispatcher */ + dco->ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl"); + + /* Register permanent callback - all messages route through ovpn_handle_msg. + * This prevents message loss when async notifications (like DEL_PEER) arrive + * while a different callback would otherwise be registered for stats requests. */ + nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco); + /* The async PACKET messages confuse libnl and it will drop them with * wrong sequence numbers (NLE_SEQ_MISMATCH), so disable libnl's sequence * number check */ @@ -495,7 +514,7 @@ dco_swap_keys(dco_context_t *dco, unsigned int peerid) NLA_PUT_U32(nl_msg, OVPN_SWAP_KEYS_ATTR_PEER_ID, peerid); nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -519,7 +538,7 @@ dco_del_peer(dco_context_t *dco, unsigned int peerid) NLA_PUT_U32(nl_msg, OVPN_DEL_PEER_ATTR_PEER_ID, peerid); nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -545,7 +564,7 @@ dco_del_key(dco_context_t *dco, unsigned int peerid, NLA_PUT_U8(nl_msg, OVPN_DEL_KEY_ATTR_KEY_SLOT, slot); nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -602,7 +621,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -631,7 +650,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid, keepalive_timeout); nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); @@ -697,37 +716,61 @@ ovpn_get_mcast_id(dco_context_t *dco) { dco->ovpn_dco_mcast_id = -ENOENT; - /* Even though 'nlctrl' is a constant, there seem to be no library - * provided define for it */ - int ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl"); - struct nl_msg *nl_msg = nlmsg_alloc(); if (!nl_msg) { return -ENOMEM; } - genlmsg_put(nl_msg, 0, 0, ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0); + /* Use ctrlid stored in dco during init */ + genlmsg_put(nl_msg, 0, 0, dco->ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0); int ret = -EMSGSIZE; NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_NL_NAME); - ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__); + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); nla_put_failure: nlmsg_free(nl_msg); return ret; } -/* This function parses any netlink message sent by ovpn-dco to userspace */ +/** + * Central message dispatcher for all netlink messages. + * + * This function is registered as the permanent NL_CB_VALID callback and routes + * messages to appropriate handlers based on type. This prevents message loss + * when async notifications (like DEL_PEER) arrive during other operations. + */ static int ovpn_handle_msg(struct nl_msg *msg, void *arg) { dco_context_t *dco = arg; + struct nlmsghdr *nlh = nlmsg_hdr(msg); + struct genlmsghdr *gnlh = nlmsg_data(nlh); - struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg)); + /* Route NLCTRL messages to mcast handler */ + if (nlh->nlmsg_type == dco->ctrlid) + { + return mcast_family_handler(msg, dco); + } + + /* Route GET_PEER responses to stats handlers */ + if (gnlh->cmd == OVPN_CMD_GET_PEER) + { + if (dco->m) + { + return dco_parse_peer_multi(msg, dco); + } + else if (dco->c) + { + return dco_parse_peer(msg, dco); + } + return NL_SKIP; + } + + /* Handle async notifications (DEL_PEER, etc.) */ struct nlattr *attrs[OVPN_ATTR_MAX + 1]; - struct nlmsghdr *nlh = nlmsg_hdr(msg); if (!genlmsg_valid_hdr(nlh, 0)) { @@ -813,15 +856,27 @@ ovpn_handle_msg(struct nl_msg *msg, void *arg) return NL_SKIP; } + /* Process each message immediately to prevent data loss when multiple + * messages arrive simultaneously. Previously, storing in dco fields and + * processing later caused only the last message to be handled. */ + if (dco->c && dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else if (dco->c) + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); - nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco); - + /* Callback permanently registered in ovpn_dco_init_netlink(). + * Each message is processed immediately inside ovpn_handle_msg(). */ return ovpn_nl_recvmsgs(dco, __func__); } @@ -877,14 +932,22 @@ dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id) } } -int +static int dco_parse_peer_multi(struct nl_msg *msg, void *arg) { + dco_context_t *dco = arg; + struct multi_context *m = dco->m; struct nlattr *tb[OVPN_ATTR_MAX + 1]; struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg)); msg(D_DCO_DEBUG, "%s: parsing message...", __func__); + if (!m) + { + msg(D_DCO_DEBUG, "%s: no multi context set", __func__); + return NL_SKIP; + } + nla_parse(tb, OVPN_ATTR_MAX, genlmsg_attrdata(gnlh, 0), genlmsg_attrlen(gnlh, 0), NULL); @@ -905,7 +968,6 @@ dco_parse_peer_multi(struct nl_msg *msg, void *arg) return NL_SKIP; } - struct multi_context *m = arg; uint32_t peer_id = nla_get_u32(tb_peer[OVPN_GET_PEER_RESP_ATTR_PEER_ID]); if (peer_id >= m->max_clients || !m->instances[peer_id]) @@ -925,11 +987,20 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) { msg(D_DCO_DEBUG, "%s", __func__); + /* Skip stats request if we're processing DCO messages to avoid recursion */ + if (__is_locked) + { + return 0; + } + struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_GET_PEER); nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP; - int ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__); + /* Store context for dispatcher to route to dco_parse_peer_multi */ + dco->m = m; + int ret = ovpn_nl_msg_send(dco, nl_msg, __func__); + dco->m = NULL; nlmsg_free(nl_msg); return ret; @@ -938,12 +1009,19 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) static int dco_parse_peer(struct nl_msg *msg, void *arg) { - struct context *c = arg; + dco_context_t *dco = arg; + struct context *c = dco->c; struct nlattr *tb[OVPN_ATTR_MAX + 1]; struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg)); msg(D_DCO_DEBUG, "%s: parsing message...", __func__); + if (!c) + { + msg(D_DCO_DEBUG, "%s: no context set", __func__); + return NL_SKIP; + } + nla_parse(tb, OVPN_ATTR_MAX, genlmsg_attrdata(gnlh, 0), genlmsg_attrlen(gnlh, 0), NULL); @@ -987,6 +1065,12 @@ dco_get_peer_stats(struct context *c) return 0; } + /* Skip stats request if we're processing DCO messages to avoid recursion */ + if (__is_locked) + { + return 0; + } + dco_context_t *dco = &c->c1.tuntap->dco; struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_GET_PEER); struct nlattr *attr = nla_nest_start(nl_msg, OVPN_ATTR_GET_PEER); @@ -995,7 +1079,10 @@ dco_get_peer_stats(struct context *c) NLA_PUT_U32(nl_msg, OVPN_GET_PEER_ATTR_PEER_ID, peer_id); nla_nest_end(nl_msg, attr); - ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer, c, __func__); + /* Store context for dispatcher to route to dco_parse_peer */ + dco->c = c; + ret = ovpn_nl_msg_send(dco, nl_msg, __func__); + dco->c = NULL; nla_put_failure: nlmsg_free(nl_msg); diff --git a/src/openvpn/dco_linux.h b/src/openvpn/dco_linux.h index 5179912b522..9d147b50531 100644 --- a/src/openvpn/dco_linux.h +++ b/src/openvpn/dco_linux.h @@ -36,6 +36,9 @@ typedef enum ovpn_key_slot dco_key_slot_t; typedef enum ovpn_cipher_alg dco_cipher_t; +/* Forward declarations for callback context */ +struct multi_context; +struct context; typedef struct { @@ -47,6 +50,7 @@ typedef struct int ovpn_dco_id; int ovpn_dco_mcast_id; + int ctrlid; /* NLCTRL family ID for message routing */ unsigned int ifindex; @@ -55,6 +59,10 @@ typedef struct int dco_del_peer_reason; uint64_t dco_read_bytes; uint64_t dco_write_bytes; + + /* Callback context for stats handlers */ + struct multi_context *m; /* For multi-peer stats */ + struct context *c; /* For single-peer stats */ } dco_context_t; #endif /* defined(ENABLE_DCO) && defined(TARGET_LINUX) */ diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index 0b8f8319de1..090e93a857b 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -423,7 +423,7 @@ dco_version_string(struct gc_arena *gc) } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { /* no-op on windows */ ASSERT(0); diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index ab5ebda968f..5d37d9182be 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1269,13 +1269,16 @@ extract_dco_float_peer_addr(const sa_family_t socket_family, } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); + /* Get context from DCO context pointer set during initialization */ + struct context *c = dco->c; + if (!c) + { + return; + } /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2344,7 +2347,7 @@ process_io(struct context *c) { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 3d0abd5af12..9f2b9f1ede5 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -50,6 +50,7 @@ #include "openvpn.h" #include "occ.h" #include "ping.h" +#include "dco.h" #define IOW_TO_TUN (1<<0) #define IOW_TO_LINK (1<<1) @@ -248,6 +249,17 @@ void read_incoming_tun(struct context *c); */ void process_incoming_tun(struct context *c); +/** + * Process an incoming DCO message (from kernel space) for P2P mode. + * + * Called immediately from the netlink callback ovpn_handle_msg() to process + * each message as it arrives. This prevents message loss when multiple + * DEL_PEER notifications arrive simultaneously. + * + * @param dco - The DCO context containing the parsed message. + */ +void process_incoming_dco(dco_context_t *dco); + /** * Write a packet to the virtual tun/tap network interface. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 1476737efbe..a2a0febab08 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1883,6 +1883,7 @@ do_open_tun(struct context *c, int *error_flags) if (dco_enabled(&c->options)) { ovpn_dco_init(c->mode, &c->c1.tuntap->dco); + c->c1.tuntap->dco.c = c; /* Link DCO context to main context for mode detection */ } /* open the tun device */ diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c index 96408d11966..3d5556d6944 100644 --- a/src/openvpn/mtcp.c +++ b/src/openvpn/mtcp.c @@ -747,7 +747,7 @@ multi_tcp_process_io(struct multi_context *m) /* incoming data on DCO? */ else if (e->arg == MTCP_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ @@ -804,6 +804,9 @@ tunnel_server_tcp(struct context *top) /* initialize global multi_context object */ multi_init(&multi, top, true); + /* Link top context back to multi_context for DCO immediate processing */ + top->multi = &multi; + /* initialize our cloned top object */ multi_top_init(&multi, top); diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index 0492311669c..4a929a771be 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -409,11 +409,8 @@ multi_process_io_udp(struct multi_context *m) { if (!IS_SIG(&m->top)) { - bool ret = true; - while (ret) - { - ret = multi_process_incoming_dco(m); - } + /* Single call handles all pending messages (processing in callback) */ + dco_read_and_process(&m->top.c1.tuntap->dco); } } #endif @@ -478,6 +475,9 @@ tunnel_server_udp(struct context *top) /* initialize global multi_context object */ multi_init(&multi, top, false); + /* Link top context back to multi_context for DCO immediate processing */ + top->multi = &multi; + /* initialize our cloned top object */ multi_top_init(&multi, top); diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 94e623ba22e..4b7ce45ada5 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3287,15 +3287,15 @@ process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi, multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + /* Get multi_context from the context pointer set during initialization */ + ASSERT(dco->c && dco->c->multi); + struct multi_context *m = dco->c->multi; struct multi_instance *mi = NULL; - int ret = dco_do_read(&m->top.c1.tuntap->dco); - int peer_id = dco->dco_message_peer_id; /* no peer-specific message delivered -> nothing to process. @@ -3303,7 +3303,7 @@ multi_process_incoming_dco(struct multi_context *m) */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) @@ -3356,7 +3356,6 @@ multi_process_incoming_dco(struct multi_context *m) dco->dco_del_peer_reason = -1; dco->dco_read_bytes = 0; dco->dco_write_bytes = 0; - return ret > 0; } #endif /* if defined(ENABLE_DCO) && defined(TARGET_LINUX) */ diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index 7167639e869..a6b7895e0b1 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -40,6 +40,7 @@ #include "perf.h" #include "vlan.h" #include "reflect_filter.h" +#include "dco.h" #define MULTI_PREFIX_MAX_LENGTH 256 @@ -317,13 +318,13 @@ bool multi_process_post(struct multi_context *m, struct multi_instance *mi, cons /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structur.e + * Called immediately from the netlink callback ovpn_handle_msg() to process + * each message as it arrives. This prevents message loss when multiple + * DEL_PEER notifications arrive simultaneously. * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - The DCO context containing the parsed message. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 9cba1c5adf3..495ee6dbbea 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -46,6 +46,9 @@ #include "plugin.h" #include "manage.h" +/* Forward declaration for multi-client context */ +struct multi_context; + /* * Our global key schedules, packaged thusly * to facilitate --persist-key. @@ -515,6 +518,9 @@ struct context struct context_0 *c0; /**< Level 0 %context. */ struct context_1 c1; /**< Level 1 %context. */ struct context_2 c2; /**< Level 2 %context. */ + + struct multi_context *multi; /**< Pointer to the main P2MP context. + * Non-NULL only when mode == CM_TOP. */ }; /*