| Message ID | 20221227022404.3468137-3-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2,1/4] Ensure we do not promote a TA_TIMEOUT to a TA_READ event with dco | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> I have not actually seen this particular race (or if I have seen it it was drowned in lots of other events) - but yeah, to fully ignore this seems better than "kill peer <x> just because we ended a peer with the very same peer-id <x> a few seconds ago". To achieve the "some other program might have sent this" original idea, one would need to add a sending PID ("we ignore everything with our own PID"), or some sort of ACK mechanism ("we do not re-assign the peer-id <x> until we have confirmation from the kernel"). This would certainly be nice, but for now, ignore it is. Tested on the ubuntu2004 DCO system (client/server), with no gremlins. Your patch has been applied to the master branch. commit 6ad66b0c2950c0d7674a5867085fef8115f61d11 (master) commit eec054a95820ffa7e548f47078ec0101d6949907 (release/2.6) Author: Arne Schwabe Date: Tue Dec 27 03:24:03 2022 +0100 Ignore OVPN_DEL_PEER_REASON_USERSPACE to avoid race conditions Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20221227022404.3468137-3-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25820.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 6c6385c6e..50d88f19a 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3243,10 +3243,19 @@ process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi, break; case OVPN_DEL_PEER_REASON_USERSPACE: - /* This very likely ourselves but might be another process, so - * still process it */ - reason = "ovpn-dco: userspace request"; - break; + /* We assume that is ourselves. UUnfortunately, sometimes these + * events happen with enough delay that they can have an order of + * + * dco_del_peer x + * [new client connecting] + * dco_new_peer x + * event from dco_del_peer arrives. + * + * if we do not ignore this we get desynced with the kernel + * since we assume the peer-id is free again. The other way would + * be to send a dco_del_peer again + */ + return; } /* When kernel already deleted the peer, the socket is no longer
Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/multi.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)