From 63edf11b13076b6c7956c326496bed1c7f7d145b Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Tue, 11 Mar 2014 11:28:40 -0700 Subject: [PATCH] Fix Windows tun read handling It's too fragile to depend on the 'new_pkt' check, and only do the ReadFile() call when we allocate a new packet buffer. Keep track of the pending state directly in the Windows-specific code, instead. This should fix the fact that it gets out of sync when we get an error. Such as the ERROR_MORE_DATA errors we get because vpnc-script-win.js doesn't set the MTU properly yet... Signed-off-by: David Woodhouse --- mainloop.c | 4 +--- openconnect-internal.h | 3 ++- tun-win32.c | 50 ++++++++++++++++++++++++------------------ tun.c | 2 +- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/mainloop.c b/mainloop.c index 30b94f2e..cac7b2dc 100644 --- a/mainloop.c +++ b/mainloop.c @@ -56,10 +56,8 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout) if (read_fd_monitored(vpninfo, tun)) { while (1) { int len = vpninfo->ip_info.mtu; - int new_pkt = 0; if (!out_pkt) { - new_pkt = 1; out_pkt = malloc(sizeof(struct pkt) + len); if (!out_pkt) { vpn_progress(vpninfo, PRG_ERR, "Allocation failed\n"); @@ -68,7 +66,7 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout) out_pkt->len = len; } - if (os_read_tun(vpninfo, out_pkt, new_pkt)) + if (os_read_tun(vpninfo, out_pkt)) break; vpninfo->stats.tx_pkts++; diff --git a/openconnect-internal.h b/openconnect-internal.h index c1abb4ea..dec8414f 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -294,6 +294,7 @@ struct openconnect_info { #ifdef _WIN32 HANDLE tun_fh; OVERLAPPED tun_rd_overlap, tun_wr_overlap; + int tun_rd_pending; #else int tun_fd; #endif @@ -453,7 +454,7 @@ int script_config_tun(struct openconnect_info *vpninfo, const char *reason); /* tun.c / tun-win32.c */ void os_shutdown_tun(struct openconnect_info *vpninfo); -int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt, int new_pkt); +int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt); int os_write_tun(struct openconnect_info *vpninfo, struct pkt *pkt); intptr_t os_setup_tun(struct openconnect_info *vpninfo); diff --git a/tun-win32.c b/tun-win32.c index 22accbc9..6ba1f33c 100644 --- a/tun-win32.c +++ b/tun-win32.c @@ -200,33 +200,41 @@ intptr_t os_setup_tun(struct openconnect_info *vpninfo) return search_taps(vpninfo, open_tun, 0); } -int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt, int new_pkt) +int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt) { DWORD pkt_size; - /* For newly-allocated packets we have to trigger the read. */ - if (new_pkt) { - if (!ReadFile(vpninfo->tun_fh, pkt->data, pkt->len, &pkt_size, &vpninfo->tun_rd_overlap)) { - DWORD err = GetLastError(); - if (err != ERROR_IO_PENDING) - vpn_progress(vpninfo, PRG_ERR, - _("Failed to read from TAP device: %lx\n"), - err); - return -1; - } - } else { - /* IF it isn't a new packet, then there was already a pending read on it. */ - if (!GetOverlappedResult(vpninfo->tun_fh, &vpninfo->tun_rd_overlap, &pkt_size, FALSE)) { - DWORD err = GetLastError(); - - if (err != ERROR_IO_INCOMPLETE) - vpn_progress(vpninfo, PRG_ERR, - _("Failed to complete read from TAP device: %lx\n"), - err); - return -1; + reread: + if (!vpninfo->tun_rd_pending && + !ReadFile(vpninfo->tun_fh, pkt->data, pkt->len, &pkt_size, + &vpninfo->tun_rd_overlap)) { + DWORD err = GetLastError(); + + if (err == ERROR_IO_PENDING) + vpninfo->tun_rd_pending = 1; + else + vpn_progress(vpninfo, PRG_ERR, + _("Failed to read from TAP device: %lx\n"), + err); + return -1; + } else if (!GetOverlappedResult(vpninfo->tun_fh, + &vpninfo->tun_rd_overlap, &pkt_size, + FALSE)) { + DWORD err = GetLastError(); + + if (err != ERROR_IO_INCOMPLETE) { + vpninfo->tun_rd_pending = 0; + vpn_progress(vpninfo, PRG_ERR, + _("Failed to complete read from TAP device: %lx\n"), + err); + goto reread; } + return -1; } + /* Either a straight ReadFile() or a subsequent GetOverlappedResult() + succeeded... */ + vpninfo->tun_rd_pending = 0; pkt->len = pkt_size; return 0; } diff --git a/tun.c b/tun.c index afb74737..d0397202 100644 --- a/tun.c +++ b/tun.c @@ -376,7 +376,7 @@ int openconnect_setup_tun_script(struct openconnect_info *vpninfo, char *tun_scr return openconnect_setup_tun_fd(vpninfo, fds[0]); } -int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt, int new_pkt) +int os_read_tun(struct openconnect_info *vpninfo, struct pkt *pkt) { int prefix_size = 0; int len;