2024-11-14, 09:12:01 +0100, Antonio Quartulli wrote:
On 13/11/2024 11:36, Sabrina Dubroca wrote:
2024-11-12, 14:20:45 +0100, Antonio Quartulli wrote:
On 05/11/2024 19:10, Sabrina Dubroca wrote:
2024-10-29, 11:47:28 +0100, Antonio Quartulli wrote:
- /* check for peer timeout */
- expired = false;
- timeout = peer->keepalive_timeout;
- delta = now - peer->last_recv;
I'm not sure that's always > 0 if we finish decrypting a packet just as the workqueue starts:
ovpn_peer_keepalive_work now = ... ovpn_decrypt_post peer->last_recv = ... ovpn_peer_keepalive_work_single delta: now < peer->last_recvYeah, there is nothing preventing this from happening...but is this truly a problem? The math should still work, no?
We'll fail "delta < timeout" (which we shouldn't), so we'll end up either in the "expired = true" case, or not updating keepalive_recv_exp. Both of these seem not ideal.
delta is signed, so it'll end up being a negative value and "delta < timeout" should not fail then. Unless I am missing something.
But timeout is "unsigned long", so the comparison will be done as unsigned.
Anyway, this was just an exercise to understand what was going on. I already changed the code as per your suggestion (the fact that we are still discussing this chunk proves that it needed to be simplified :))
:)