tftpd: some changes

[Maxime Villard <max%M00nBSD.net@localhost>]

Hi,
here is a patch for tftpd.

- - CVS
The functions tsize_handler()/timeout_handler()/blk_handler() are
given a pointer 'ec' that they are *supposed* to change when an error
occurs. Also, they are *supposed* to return -1 to indicate that an
error occured. But, actually, they never change 'ec' and almost
never return -1. It means that if get_options() fails, nak() will
send a wrong error message.

- - PATCH
Those three functions now return -1 when an invalid option is received,
and they set the error code as appropriate. Since I *think* it should
be considered as an illegal TFTP operation, I've put EBADOP. Also,
in get_options(), I removed the variable 'ec'; it's better to pass
directly 'ecode'. Thus, I renamed all the 'ec' to 'ecode'.

- - REMARK
At l.716, what's the goal of 'oack_h' ?

Ok/Comments?


- - - -

Index: tftpd.c
===================================================================
RCS file: /cvsroot/src/libexec/tftpd/tftpd.c,v
retrieving revision 1.40
diff -u -r1.40 tftpd.c
--- tftpd.c     28 Jun 2013 17:20:15 -0000      1.40
+++ tftpd.c     3 Jul 2013 07:05:33 -0000
@@ -420,7 +420,7 @@
 
 static int
 blk_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
-    size_t *ackl, int *ec)
+    size_t *ackl, int *ecode)
 {
        unsigned long bsize;
        char *endp;
@@ -438,7 +438,8 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
        if (bsize < 8 || bsize > 65464) {
                syslog(LOG_NOTICE, "%s: %s request for %s: "
@@ -446,22 +447,25 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
 
        tftp_blksize = bsize;
        if (asize > *ackl && (l = snprintf(ack + *ackl, asize - *ackl,
-           "blksize%c%lu%c", 0, bsize, 0)) > 0)
+           "blksize%c%lu%c", 0, bsize, 0)) > 0) {
                *ackl += l;
-       else
+       } else {
+               *ecode = EBADOP;
                return -1;
+       }
 
        return 0;
 }
 
 static int
 timeout_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
-               size_t *ackl, int *ec)
+               size_t *ackl, int *ecode)
 {
        unsigned long tout;
        char *endp;
@@ -475,7 +479,8 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
        if (tout < 1 || tout > 255) {
                syslog(LOG_NOTICE, "%s: %s request for %s: "
@@ -483,15 +488,18 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
 
        rexmtval = tout;
        if (asize > *ackl && (l = snprintf(ack + *ackl, asize - *ackl,
-           "timeout%c%lu%c", 0, tout, 0)))
+           "timeout%c%lu%c", 0, tout, 0))) {
                *ackl += l;
-       else
+       } else {
+               *ecode = EBADOP;
                return -1;
+       }
        /*
         * Arbitrarily pick a maximum timeout on a request to 3
         * retransmissions if the interval timeout is more than
@@ -509,7 +517,7 @@
 
 static int
 tsize_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
-    size_t *ackl, int *ec)
+    size_t *ackl, int *ecode)
 {
        unsigned long fsize;
        char *endp;
@@ -528,7 +536,8 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
        if (fsize > (unsigned long) 65535 * 65464) {
                syslog(LOG_NOTICE, "%s: %s request for %s: "
@@ -536,7 +545,8 @@
                        verifyhost((struct sockaddr *)&from),
                        tp->th_opcode == WRQ ? "write" : "read",
                        tp->th_stuff, val);
-               return 0;
+               *ecode = EBADOP;
+               return -1;
        }
 
        tftp_opt_tsize = 1;
@@ -566,11 +576,11 @@
  */
 static int
 get_options(struct tftphdr *tp, char *cp, int size, char *ackb, size_t asize,
-    size_t *alen, int *err)
+    size_t *alen, int *ecode)
 {
        const struct tftp_options *op;
        char *option, *value, *endp;
-       int r, rv=0, ec=0;
+       int r, rv=0;
 
        endp = cp + size;
        while (cp < endp) {
@@ -598,7 +608,7 @@
                                break;
                }
                if (op->o_name) {
-                       r = op->o_handler(tp, value, ackb, asize, alen, &ec);
+                       r = op->o_handler(tp, value, ackb, asize, alen, ecode);
                        if (r < 0) {
                                rv = -1;
                                break;
@@ -606,9 +616,6 @@
                        rv++;
                } /* else ignore unknown options */
        }
-       
-       if (rv < 0)
-               *err = ec;
 
        return rv;
 }