From f609e53c510be5e2e499e0dfdb867acc62440059 Mon Sep 17 00:00:00 2001 From: David Gwynne Date: Tue, 10 Jul 2012 07:21:35 +0000 Subject: libexec/tftpd handled the case where we'd get an ack for the previous block by flushing the data on the socket and waiting for a retransmit by timeout to occur. my stuff (usr.sbin/tftpd) had XXXs and failure in those places. this diff fixes that. this should address the problems that sthen and jcs have been having (and which i was finally able to reproduce here). it also avoids reusing the clients buffer to both send and recv frames. we recv onto the stack now so retry always sends what we originally built for the peer. tested by and ok jcs@ sthen@ --- usr.sbin/tftpd/tftpd.c | 108 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 19 deletions(-) (limited to 'usr.sbin/tftpd/tftpd.c') diff --git a/usr.sbin/tftpd/tftpd.c b/usr.sbin/tftpd/tftpd.c index 52df5c13447..70a82bba83e 100644 --- a/usr.sbin/tftpd/tftpd.c +++ b/usr.sbin/tftpd/tftpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tftpd.c,v 1.5 2012/03/15 07:18:35 nicm Exp $ */ +/* $OpenBSD: tftpd.c,v 1.6 2012/07/10 07:21:34 dlg Exp $ */ /* * Copyright (c) 2012 David Gwynne @@ -162,6 +162,7 @@ int tftpd_listen(const char *, const char *, int); void tftpd_events(void); void tftpd_recv(int, short, void *); int retry(struct tftp_client *); +int tftp_flush(struct tftp_client *); void tftp(struct tftp_client *, struct tftphdr *, size_t); void tftp_open(struct tftp_client *, const char *); @@ -1077,16 +1078,19 @@ tftp_rrq_ack(int fd, short events, void *arg) { struct tftp_client *client = arg; struct tftphdr *ap; /* ack packet */ + char rbuf[SEGSIZE_MIN]; ssize_t n; if (events & EV_TIMEOUT) { - if (retry(client) == -1) + if (retry(client) == -1) { + lwarn("%s: retry", getip(&client->ss)); goto done; + } return; } - n = recv(fd, client->buf, client->packet_size, 0); + n = recv(fd, rbuf, sizeof(rbuf), 0); if (n == -1) { switch (errno) { case EINTR: @@ -1095,18 +1099,35 @@ tftp_rrq_ack(int fd, short events, void *arg) return; default: + lwarn("%s: recv", getip(&client->ss)); goto done; } } - ap = (struct tftphdr *)client->buf; + ap = (struct tftphdr *)rbuf; ap->th_opcode = ntohs((u_short)ap->th_opcode); ap->th_block = ntohs((u_short)ap->th_block); - if (ap->th_opcode == ERROR) + switch (ap->th_opcode) { + case ERROR: goto done; - if (ap->th_opcode != ACK || ap->th_block != client->block) - goto done; /* XXX */ + case ACK: + break; + default: + goto retry; + } + + if (ap->th_block != client->block) { + if (tftp_flush(client) == -1) { + lwarnx("%s: flush", getip(&client->ss)); + goto done; + } + + if (ap->th_block != (client->block - 1)) + goto done; + + goto retry; + } if (client->buflen != client->packet_size) { /* this was the last packet in the stream */ @@ -1117,10 +1138,37 @@ tftp_rrq_ack(int fd, short events, void *arg) file_read(client); return; +retry: + event_add(&client->sev, &client->tv); + return; + done: client_free(client); } +int +tftp_flush(struct tftp_client *client) +{ + char rbuf[SEGSIZE_MIN]; + ssize_t n; + + for (;;) { + n = recv(client->sock, rbuf, sizeof(rbuf), 0); + if (n == -1) { + switch (errno) { + case EAGAIN: + return (0); + + case EINTR: + break; + + default: + return (-1); + } + } + } +} + void recvfile(struct tftp_client *client) { @@ -1137,8 +1185,8 @@ tftp_wrq_ack_packet(struct tftp_client *client) ap->th_opcode = htons((u_short)ACK); ap->th_block = htons(client->block); - client->retries = RETRIES; client->buflen = 4; + client->retries = RETRIES; return (send(client->sock, client->buf, client->buflen, 0) != 4); } @@ -1159,19 +1207,22 @@ tftp_wrq_ack(struct tftp_client *client) void tftp_wrq(int fd, short events, void *arg) { + char wbuf[SEGSIZE_MAX + 4]; struct tftp_client *client = arg; struct tftphdr *dp; ssize_t n; int i; if (events & EV_TIMEOUT) { - if (retry(client) == -1) + if (retry(client) == -1) { + lwarn("%s", getip(&client->ss)); goto done; + } return; } - n = recv(client->sock, client->buf, client->packet_size, 0); + n = recv(fd, wbuf, client->packet_size, 0); if (n == -1) { switch (errno) { case EINTR: @@ -1187,18 +1238,33 @@ tftp_wrq(int fd, short events, void *arg) if (n < 4) goto done; - dp = (struct tftphdr *)client->buf; + dp = (struct tftphdr *)wbuf; dp->th_opcode = ntohs((u_short)dp->th_opcode); dp->th_block = ntohs((u_short)dp->th_block); - if (dp->th_opcode != DATA) + switch (dp->th_opcode) { + case ERROR: goto done; + case DATA: + break; + default: + goto retry; + } - if (dp->th_block != client->block) - goto done; + if (dp->th_block != client->block) { + if (tftp_flush(client) == -1) { + lwarnx("%s: flush", getip(&client->ss)); + goto done; + } + + if (dp->th_block != (client->block - 1)) + goto done; + + goto retry; + } for (i = 4; i < n; i++) { - if (client->fputc(client, client->buf[i]) == EOF) { + if (client->fputc(client, wbuf[i]) == EOF) { lwarn("tftp wrq"); goto done; } @@ -1317,11 +1383,13 @@ error: int retry(struct tftp_client *client) { - if (--client->retries == 0) + if (--client->retries == 0) { + errno = ETIMEDOUT; return (-1); + } if (send(client->sock, client->buf, client->buflen, 0) == -1) - return -1; + return (-1); event_add(&client->sev, &client->tv); @@ -1336,8 +1404,10 @@ oack_done(int fd, short events, void *arg) ssize_t n; if (events & EV_TIMEOUT) { - if (retry(client) == -1) + if (retry(client) == -1) { + lwarn("%s", getip(&client->ss)); goto done; + } return; } @@ -1351,7 +1421,7 @@ oack_done(int fd, short events, void *arg) return; default: - lwarn("recv"); + lwarn("%s: recv", getip(&client->ss)); goto done; } } -- cgit v1.2.3