Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 <David.Woodhouse@intel.com>
  • Loading branch information
David Woodhouse authored and David Woodhouse committed Mar 11, 2014
1 parent 616deb6 commit 63edf11
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 16 deletions.
4 changes: 1 addition & 3 deletions mainloop.c
Expand Up @@ -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");
Expand All @@ -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++;
Expand Down
3 changes: 2 additions & 1 deletion openconnect-internal.h
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
30 changes: 19 additions & 11 deletions tun-win32.c
Expand Up @@ -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)) {
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)

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 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)) {
} else if (!GetOverlappedResult(vpninfo->tun_fh,
&vpninfo->tun_rd_overlap, &pkt_size,
FALSE)) {
DWORD err = GetLastError();

if (err != ERROR_IO_INCOMPLETE)
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);
return -1;
goto reread;
}
return -1;
}

/* Either a straight ReadFile() or a subsequent GetOverlappedResult()
succeeded... */
vpninfo->tun_rd_pending = 0;
pkt->len = pkt_size;
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion tun.c
Expand Up @@ -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;
Expand Down

0 comments on commit 63edf11

Please sign in to comment.