From e500154b7ee1d8c6b609784a86a816046b94e854 Mon Sep 17 00:00:00 2001 From: Theo de Raadt Date: Tue, 25 Apr 2006 16:14:28 +0000 Subject: kill setjmp() and alarm() methods, which are almost always inverted signal races. use poll() instead. by marcus@nazgul.ch --- libexec/tftpd/tftpd.c | 292 +++++++++++++++++++++++++++++--------------------- 1 file changed, 172 insertions(+), 120 deletions(-) (limited to 'libexec') diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c index c41adf81f20..4c08a3012d2 100644 --- a/libexec/tftpd/tftpd.c +++ b/libexec/tftpd/tftpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tftpd.c,v 1.42 2006/04/17 08:42:05 deraadt Exp $ */ +/* $OpenBSD: tftpd.c,v 1.43 2006/04/25 16:14:27 deraadt Exp $ */ /* * Copyright (c) 1983 Regents of the University of California. @@ -37,7 +37,7 @@ char copyright[] = #ifndef lint /*static char sccsid[] = "from: @(#)tftpd.c 5.13 (Berkeley) 2/26/91";*/ -static char rcsid[] = "$OpenBSD: tftpd.c,v 1.42 2006/04/17 08:42:05 deraadt Exp $"; +static char rcsid[] = "$OpenBSD: tftpd.c,v 1.43 2006/04/25 16:14:27 deraadt Exp $"; #endif /* not lint */ /* @@ -50,9 +50,7 @@ static char rcsid[] = "$OpenBSD: tftpd.c,v 1.42 2006/04/17 08:42:05 deraadt Exp #include #include #include -#include #include -#include #include #include @@ -60,7 +58,7 @@ static char rcsid[] = "$OpenBSD: tftpd.c,v 1.42 2006/04/17 08:42:05 deraadt Exp #include #include -#include +#include #include #include #include @@ -94,6 +92,8 @@ int validate_access(char *filename, int mode); int recvfile(struct formats *pf); int sendfile(struct formats *pf); +FILE *file; + struct formats { const char *f_mode; int (*f_validate)(char *, int); @@ -119,6 +119,22 @@ enum opt_enum { OPT_TIMEOUT }; +struct errmsg { + int e_code; + const char *e_msg; +} errmsgs[] = { + { EUNDEF, "Undefined error code" }, + { ENOTFOUND, "File not found" }, + { EACCESS, "Access violation" }, + { ENOSPACE, "Disk full or allocation exceeded" }, + { EBADOP, "Illegal TFTP operation" }, + { EBADID, "Unknown transfer ID" }, + { EEXISTS, "File already exists" }, + { ENOUSER, "No such user" }, + { EOPTNEG, "Option negotiation failed" }, + { -1, NULL } +}; + int validate_access(char *filename, int mode); void tftp(struct tftphdr *tp, int size); void nak(int error); @@ -447,9 +463,6 @@ option_fail: exit(0); } - -FILE *file; - /* * Validate file access. Since we * have no uid or gid, for now require @@ -538,19 +551,6 @@ validate_access(char *filename, int mode) return (0); } -int timeouts; -jmp_buf timeoutbuf; - -/* ARGSUSED */ -static void -timer(int signo) -{ - /* XXX longjmp/signal resource leaks */ - if (++timeouts >= MAX_TIMEOUTS) - _exit(1); - longjmp(timeoutbuf, 1); -} - /* * Send the requested file. */ @@ -558,14 +558,16 @@ int sendfile(struct formats *pf) { struct tftphdr *dp, *r_init(void); - struct tftphdr *ap; /* ack packet */ + struct tftphdr *ap; /* ack packet */ + struct pollfd pfd[1]; volatile unsigned short block = 1; - int size, n; + int n, size, nfds, error, timeouts; - signal(SIGALRM, timer); dp = r_init(); ap = (struct tftphdr *)ackbuf; + do { + /* read data from file */ size = readit(file, &dp, pf->f_convert); if (size < 0) { nak(errno + 100); @@ -573,21 +575,41 @@ sendfile(struct formats *pf) } dp->th_opcode = htons((u_short)DATA); dp->th_block = htons((u_short)block); - timeouts = 0; - setjmp(timeoutbuf); -send_data: - if (send(peer, dp, size + 4, 0) != size + 4) { - syslog(LOG_ERR, "tftpd: write: %m"); - goto abort; - } - read_ahead(file, pf->f_convert); - for ( ; ; ) { - alarm(rexmtval); /* read the ack */ - n = recv(peer, ackbuf, sizeof (ackbuf), 0); - alarm(0); - if (n < 0) { - syslog(LOG_ERR, "tftpd: read: %m"); + /* send data to client and wait for client ACK */ + for (timeouts = 0, error = 0;;) { + if (timeouts == MAX_TIMEOUTS) + exit(1); + + if (!error) { + if (send(peer, dp, size + 4, 0) != size + 4) { + syslog(LOG_ERR, "tftpd: send: %m"); + goto abort; + } + read_ahead(file, pf->f_convert); + } + error = 0; + + pfd[0].fd = peer; + pfd[0].events = POLLIN; + nfds = poll(pfd, 1, TIMEOUT * 1000); + if (nfds == 0) { + timeouts++; + continue; + } + if (nfds == -1) { + error = 1; + if (errno == EINTR) + continue; + syslog(LOG_ERR, "tftpd: poll: %m"); + goto abort; + } + n = recv(peer, ackbuf, sizeof(ackbuf), 0); + if (n == -1) { + error = 1; + if (errno == EINTR) + continue; + syslog(LOG_ERR, "tftpd: recv: %m"); goto abort; } ap->th_opcode = ntohs((u_short)ap->th_opcode); @@ -595,34 +617,25 @@ send_data: if (ap->th_opcode == ERROR) goto abort; - if (ap->th_opcode == ACK) { - if (ap->th_block == block) { + if (ap->th_block == block) break; - } - /* Re-synchronize with the other side */ + /* re-synchronize with the other side */ (void) synchnet(peer); - if (ap->th_block == (block -1)) { - goto send_data; - } + if (ap->th_block == (block - 1)) + continue; } - + error = 1; /* FALLTHROUGH */ } + block++; } while (size == SEGSIZE); + abort: fclose(file); return (1); } -/* ARGSUSED */ -static void -justquit(int signo) -{ - _exit(0); -} - - /* * Receive a file. */ @@ -630,50 +643,75 @@ int recvfile(struct formats *pf) { struct tftphdr *dp, *w_init(void); - struct tftphdr *ap; /* ack buffer */ + struct tftphdr *ap; /* ack buffer */ + struct pollfd pfd[1]; volatile unsigned short block = 0; - int n, size; + int n, size, nfds, error, timeouts; - signal(SIGALRM, timer); dp = w_init(); ap = (struct tftphdr *)ackbuf; + do { - timeouts = 0; + /* create new ACK packet */ ap->th_opcode = htons((u_short)ACK); ap->th_block = htons((u_short)block); block++; - setjmp(timeoutbuf); -send_ack: - if (send(peer, ackbuf, 4, 0) != 4) { - syslog(LOG_ERR, "tftpd: write: %m"); - goto abort; - } - write_behind(file, pf->f_convert); - for ( ; ; ) { - alarm(rexmtval); + + /* send ACK to client and wait for client data */ + for (timeouts = 0, error = 0;;) { + if (timeouts == MAX_TIMEOUTS) + exit(1); + + if (!error) { + if (send(peer, ackbuf, 4, 0) != 4) { + syslog(LOG_ERR, "tftpd: send: %m"); + goto abort; + } + write_behind(file, pf->f_convert); + } + error = 0; + + pfd[0].fd = peer; + pfd[0].events = POLLIN; + nfds = poll(pfd, 1, TIMEOUT * 1000); + if (nfds == 0) { + timeouts++; + continue; + } + if (nfds == -1) { + error = 1; + if (errno == EINTR) + continue; + syslog(LOG_ERR, "tftpd: poll: %m"); + goto abort; + } n = recv(peer, dp, PKTSIZE, 0); - alarm(0); - if (n < 0) { /* really? */ - syslog(LOG_ERR, "tftpd: read: %m"); + if (n == -1) { + error = 1; + if (errno == EINTR) + continue; + syslog(LOG_ERR, "tftpd: recv: %m"); goto abort; } dp->th_opcode = ntohs((u_short)dp->th_opcode); dp->th_block = ntohs((u_short)dp->th_block); + if (dp->th_opcode == ERROR) goto abort; if (dp->th_opcode == DATA) { - if (dp->th_block == block) { - break; /* normal */ - } - /* Re-synchronize with the other side */ + if (dp->th_block == block) + break; + /* re-synchronize with the other side */ (void) synchnet(peer); - if (dp->th_block == (block-1)) - goto send_ack; /* rexmit */ + if (dp->th_block == (block - 1)) + continue; } + error = 1; /* FALLTHROUGH */ } - /* size = write(file, dp->th_data, n - 4); */ + + /* write data to file */ size = writeit(file, &dp, n - 4, pf->f_convert); - if (size != (n-4)) { /* ahem */ + if (size != (n - 4)) { if (size < 0) nak(errno + 100); else @@ -681,42 +719,34 @@ send_ack: goto abort; } } while (size == SEGSIZE); + + /* close data file */ write_behind(file, pf->f_convert); - (void) fclose(file); /* close data file */ + (void) fclose(file); - ap->th_opcode = htons((u_short)ACK); /* send the "final" ack */ + /* send final ack */ + ap->th_opcode = htons((u_short)ACK); ap->th_block = htons((u_short)(block)); (void) send(peer, ackbuf, 4, 0); - signal(SIGALRM, justquit); /* just quit on timeout */ - alarm(rexmtval); - n = recv(peer, buf, sizeof (buf), 0); /* normally times out and quits */ - alarm(0); - if (n >= 4 && /* if read some data */ - dp->th_opcode == DATA && /* and got a data block */ - block == dp->th_block) { /* then my last ack was lost */ - (void) send(peer, ackbuf, 4, 0); /* resend final ack */ - } + /* just quit on timeout */ + pfd[0].fd = peer; + pfd[0].events = POLLIN; + nfds = poll(pfd, 1, TIMEOUT * 1000); + if (nfds < 1) + exit(1); + n = recv(peer, buf, sizeof(buf), 0); + /* + * if read some data and got a data block then my last ack was lost + * resend final ack + */ + if (n >= 4 && dp->th_opcode == DATA && block == dp->th_block) + (void) send(peer, ackbuf, 4, 0); + abort: return (1); } -struct errmsg { - int e_code; - const char *e_msg; -} errmsgs[] = { - { EUNDEF, "Undefined error code" }, - { ENOTFOUND, "File not found" }, - { EACCESS, "Access violation" }, - { ENOSPACE, "Disk full or allocation exceeded" }, - { EBADOP, "Illegal TFTP operation" }, - { EBADID, "Unknown transfer ID" }, - { EEXISTS, "File already exists" }, - { ENOUSER, "No such user" }, - { EOPTNEG, "Option negotiation failed" }, - { -1, NULL } -}; - /* * Send a nak packet (error message). * Error code passed in is one of the @@ -754,8 +784,9 @@ void oack(void) { struct tftphdr *tp, *ap; - int size, i, n; + struct pollfd pfd[1]; char *bp; + int i, n, size, nfds, error, timeouts; tp = (struct tftphdr *)buf; bp = buf + 2; @@ -769,8 +800,8 @@ oack(void) syslog(LOG_ERR, "oack: no buffer space"); exit(1); } - bp += n+1; - size -= n+1; + bp += n + 1; + size -= n + 1; if (size < 0) { syslog(LOG_ERR, "oack: no buffer space"); exit(1); @@ -779,28 +810,49 @@ oack(void) } size = bp - buf; ap = (struct tftphdr *)ackbuf; - signal(SIGALRM, timer); - timeouts = 0; - (void)setjmp(timeoutbuf); - if (send(peer, buf, size, 0) != size) { - syslog(LOG_INFO, "oack: %m"); - exit(1); - } + /* send OACK to client and wait for client ACK */ + for (timeouts = 0, error = 0;;) { + if (timeouts == MAX_TIMEOUTS) + exit(1); - for (;;) { - alarm(rexmtval); - n = recv(peer, ackbuf, sizeof (ackbuf), 0); - alarm(0); - if (n < 0) { + if (!error) { + if (send(peer, buf, size, 0) != size) { + syslog(LOG_INFO, "oack: %m"); + exit(1); + } + } + error = 0; + + pfd[0].fd = peer; + pfd[0].events = POLLIN; + nfds = poll(pfd, 1, TIMEOUT * 1000); + if (nfds == 0) { + timeouts++; + continue; + } + if (nfds == -1) { + error = 1; + if (errno == EINTR) + continue; + syslog(LOG_ERR, "poll: %m"); + exit(1); + } + n = recv(peer, ackbuf, sizeof(ackbuf), 0); + if (n == -1) { + error = 1; + if (errno == EINTR) + continue; syslog(LOG_ERR, "recv: %m"); exit(1); } ap->th_opcode = ntohs((u_short)ap->th_opcode); ap->th_block = ntohs((u_short)ap->th_block); + if (ap->th_opcode == ERROR) exit(1); if (ap->th_opcode == ACK && ap->th_block == 0) break; + error = 1; /* FALLTHROUGH */ } } -- cgit v1.2.3