udhcp: new config option "Rewrite the lease file at every new acknowledge"

       (Mats Erik Andersson <mats@blue2net.com> (Blue2Net AB))
udhcp: consistently treat server_config.start/end IPs as host-order
       fix IP parsing for 64bit machines
       fix unsafe hton macro usage in read_opt()
       do not chdir("/") when daemonizing
       fix help text

1 files changed, 92 insertions, 69 deletions

diff --git a/networking/udhcp/files.c b/networking/udhcp/files.c
index 41c8717cd..7fc734889 100644
--- a/networking/udhcp/files.c
+++ b/networking/udhcp/files.c
@@ -11,42 +11,30 @@
 #include "options.h"
 
 
-/*
- * Domain names may have 254 chars, and string options can be 254
- * chars long. However, 80 bytes will be enough for most, and won't
- * hog up memory. If you have a special application, change it
- */
-#define READ_CONFIG_BUF_SIZE 80
-
-/* on these functions, make sure you datatype matches */
+/* on these functions, make sure your datatype matches */
 static int read_ip(const char *line, void *arg)
 {
         len_and_sockaddr *lsa;
-        int retval = 0;
 
         lsa = host_and_af2sockaddr(line, 0, AF_INET);
         if (lsa) {
-                *(struct in_addr*)arg = lsa->sin.sin_addr;
+                *(uint32_t*)arg = lsa->sin.sin_addr.s_addr;
                 free(lsa);
-                retval = 1;
+                return 1;
         }
-        return retval;
+        return 0;
 }
 
 static int read_mac(const char *line, void *arg)
 {
         uint8_t *mac_bytes = arg;
         struct ether_addr *temp_ether_addr;
-        int retval = 1;
 
         temp_ether_addr = ether_aton(line);
-
         if (temp_ether_addr == NULL)
-                retval = 0;
-        else
-                memcpy(mac_bytes, temp_ether_addr, 6);
-
-        return retval;
+                return 0;
+        memcpy(mac_bytes, temp_ether_addr, 6);
+        return 1;
 }
 
 
@@ -56,14 +44,13 @@ static int read_str(const char *line, void *arg)
 
         free(*dest);
         *dest = xstrdup(line);
-
         return 1;
 }
 
 
 static int read_u32(const char *line, void *arg)
 {
-        *((uint32_t*)arg) = bb_strtou32(line, NULL, 10);
+        *(uint32_t*)arg = bb_strtou32(line, NULL, 10);
         return errno == 0;
 }
 
@@ -71,15 +58,16 @@ static int read_u32(const char *line, void *arg)
 static int read_yn(const char *line, void *arg)
 {
         char *dest = arg;
-        int retval = 1;
 
-        if (!strcasecmp("yes", line))
+        if (!strcasecmp("yes", line)) {
                 *dest = 1;
-        else if (!strcasecmp("no", line))
+                return 1;
+        }
+        if (!strcasecmp("no", line)) {
                 *dest = 0;
-        else retval = 0;
-
-        return retval;
+                return 1;
+        }
+        return 0;
 }
 
 
@@ -89,8 +77,9 @@ struct option_set *find_option(struct option_set *opt_list, char code)
         while (opt_list && opt_list->data[OPT_CODE] < code)
                 opt_list = opt_list->next;
 
-        if (opt_list && opt_list->data[OPT_CODE] == code) return opt_list;
-        else return NULL;
+        if (opt_list && opt_list->data[OPT_CODE] == code)
+                return opt_list;
+        return NULL;
 }
 
 
@@ -111,7 +100,7 @@ static void attach_option(struct option_set **opt_list,
 #endif
 
                 /* make a new option */
-                new = xmalloc(sizeof(struct option_set));
+                new = xmalloc(sizeof(*new));
                 new->data = xmalloc(length + 2);
                 new->data[OPT_CODE] = option->code;
                 new->data[OPT_LEN] = length;
@@ -184,7 +173,7 @@ static int read_opt(const char *const_line, void *arg)
                         return 0;
                 if (!strcasecmp(option->name, opt))
                         break;
-                 option++;
+                option++;
         }
 
         do {
@@ -199,8 +188,11 @@ static int read_opt(const char *const_line, void *arg)
                         break;
                 case OPTION_IP_PAIR:
                         retval = read_ip(val, buffer);
-                        if (!(val = strtok(NULL, ", \t/-"))) retval = 0;
-                        if (retval) retval = read_ip(val, buffer + 4);
+                        val = strtok(NULL, ", \t/-");
+                        if (!val)
+                                retval = 0;
+                        if (retval)
+                                retval = read_ip(val, buffer + 4);
                         break;
                 case OPTION_STRING:
 #if ENABLE_FEATURE_RFC3397
@@ -220,22 +212,33 @@ static int read_opt(const char *const_line, void *arg)
                         buffer[0] = strtoul(val, &endptr, 0);
                         retval = (endptr[0] == '\0');
                         break;
-                case OPTION_U16:
-                        *result_u16 = htons(strtoul(val, &endptr, 0));
-                        retval = (endptr[0] == '\0');
+                /* htonX are macros in older libc's, using temp var
+                 * in code below for safety */
+                /* TODO: use bb_strtoX? */
+                case OPTION_U16: {
+                        unsigned long tmp = strtoul(val, &endptr, 0);
+                        *result_u16 = htons(tmp);
+                        retval = (endptr[0] == '\0' /*&& tmp < 0x10000*/);
                         break;
-                case OPTION_S16:
-                        *result_u16 = htons(strtol(val, &endptr, 0));
+                }
+                case OPTION_S16: {
+                        long tmp = strtol(val, &endptr, 0);
+                        *result_u16 = htons(tmp);
                         retval = (endptr[0] == '\0');
                         break;
-                case OPTION_U32:
-                        *result_u32 = htonl(strtoul(val, &endptr, 0));
+                }
+                case OPTION_U32: {
+                        unsigned long tmp = strtoul(val, &endptr, 0);
+                        *result_u32 = htonl(tmp);
                         retval = (endptr[0] == '\0');
                         break;
-                case OPTION_S32:
-                        *result_u32 = htonl(strtol(val, &endptr, 0));
+                }
+                case OPTION_S32: {
+                        long tmp = strtol(val, &endptr, 0);
+                        *result_u32 = htonl(tmp);
                         retval = (endptr[0] == '\0');
                         break;
+                }
                 default:
                         break;
                 }
@@ -253,7 +256,6 @@ static int read_staticlease(const char *const_line, void *arg)
         uint8_t *mac_bytes;
         uint32_t *ip;
 
-
         /* Allocate memory for addresses */
         mac_bytes = xmalloc(sizeof(unsigned char) * 8);
         ip = xmalloc(sizeof(uint32_t));
@@ -275,39 +277,54 @@ static int read_staticlease(const char *const_line, void *arg)
 }
 
 
+struct config_keyword {
+        const char *keyword;
+        int (*handler)(const char *line, void *var);
+        void *var;
+        const char *def;
+};
+
 static const struct config_keyword keywords[] = {
-        /* keyword      handler   variable address              default */
-        {"start",       read_ip,  &(server_config.start),       "192.168.0.20"},
-        {"end",         read_ip,  &(server_config.end),         "192.168.0.254"},
-        {"interface",   read_str, &(server_config.interface),   "eth0"},
-        {"option",      read_opt, &(server_config.options),     ""},
-        {"opt",         read_opt, &(server_config.options),     ""},
-        {"max_leases",  read_u32, &(server_config.max_leases),  "254"},
-        {"remaining",   read_yn,  &(server_config.remaining),   "yes"},
-        {"auto_time",   read_u32, &(server_config.auto_time),   "7200"},
-        {"decline_time",read_u32, &(server_config.decline_time),"3600"},
-        {"conflict_time",read_u32,&(server_config.conflict_time),"3600"},
-        {"offer_time",  read_u32, &(server_config.offer_time),  "60"},
-        {"min_lease",   read_u32, &(server_config.min_lease),   "60"},
-        {"lease_file",  read_str, &(server_config.lease_file),  LEASES_FILE},
-        {"pidfile",     read_str, &(server_config.pidfile),     "/var/run/udhcpd.pid"},
-        {"notify_file", read_str, &(server_config.notify_file), ""},
-        {"siaddr",      read_ip,  &(server_config.siaddr),      "0.0.0.0"},
-        {"sname",       read_str, &(server_config.sname),       ""},
-        {"boot_file",   read_str, &(server_config.boot_file),   ""},
-        {"static_lease",read_staticlease, &(server_config.static_leases),       ""},
-        /*ADDME: static lease */
-        {"",            NULL,     NULL,                         ""}
+        /* keyword       handler   variable address               default */
+        {"start",        read_ip,  &(server_config.start_ip),     "192.168.0.20"},
+        {"end",          read_ip,  &(server_config.end_ip),       "192.168.0.254"},
+        {"interface",    read_str, &(server_config.interface),    "eth0"},
+        {"option",       read_opt, &(server_config.options),      ""},
+        {"opt",          read_opt, &(server_config.options),      ""},
+        /* Avoid "max_leases value not sane" warning by setting default
+         * to default_end_ip - default_start_ip + 1: */
+        {"max_leases",   read_u32, &(server_config.max_leases),   "235"},
+        {"remaining",    read_yn,  &(server_config.remaining),    "yes"},
+        {"auto_time",    read_u32, &(server_config.auto_time),    "7200"},
+        {"decline_time", read_u32, &(server_config.decline_time), "3600"},
+        {"conflict_time",read_u32, &(server_config.conflict_time),"3600"},
+        {"offer_time",   read_u32, &(server_config.offer_time),   "60"},
+        {"min_lease",    read_u32, &(server_config.min_lease),    "60"},
+        {"lease_file",   read_str, &(server_config.lease_file),   LEASES_FILE},
+        {"pidfile",      read_str, &(server_config.pidfile),      "/var/run/udhcpd.pid"},
+        {"notify_file",  read_str, &(server_config.notify_file),  ""},
+        {"siaddr",       read_ip,  &(server_config.siaddr),       "0.0.0.0"},
+        {"sname",        read_str, &(server_config.sname),        ""},
+        {"boot_file",    read_str, &(server_config.boot_file),    ""},
+        {"static_lease", read_staticlease, &(server_config.static_leases), ""},
+        /* ADDME: static lease */
 };
 
 
+/*
+ * Domain names may have 254 chars, and string options can be 254
+ * chars long. However, 80 bytes will be enough for most, and won't
+ * hog up memory. If you have a special application, change it
+ */
+#define READ_CONFIG_BUF_SIZE 80
+
 int read_config(const char *file)
 {
         FILE *in;
         char buffer[READ_CONFIG_BUF_SIZE], *token, *line;
         int i, lm = 0;
 
-        for (i = 0; keywords[i].keyword[0]; i++)
+        for (i = 0; i < ARRAY_SIZE(keywords); i++)
                 if (keywords[i].def[0])
                         keywords[i].handler(keywords[i].def, keywords[i].var);
 
@@ -337,7 +354,7 @@ int read_config(const char *file)
                 while (i >= 0 && isspace(line[i]))
                         line[i--] = '\0';
 
-                for (i = 0; keywords[i].keyword[0]; i++)
+                for (i = 0; i < ARRAY_SIZE(keywords); i++)
                         if (!strcasecmp(token, keywords[i].keyword))
                                 if (!keywords[i].handler(line, keywords[i].var)) {
                                         bb_error_msg("cannot parse line %d of %s", lm, file);
@@ -348,6 +365,10 @@ int read_config(const char *file)
                                 }
         }
         fclose(in);
+
+        server_config.start_ip = ntohl(server_config.start_ip);
+        server_config.end_ip = ntohl(server_config.end_ip);
+
         return 1;
 }
 
@@ -408,9 +429,11 @@ void read_leases(const char *file)
          && full_read(fp, &lease, sizeof(lease)) == sizeof(lease)
         ) {
                 /* ADDME: is it a static lease */
-                if (lease.yiaddr >= server_config.start && lease.yiaddr <= server_config.end) {
+                uint32_t y = ntohl(lease.yiaddr);
+                if (y >= server_config.start_ip && y <= server_config.end_ip) {
                         lease.expires = ntohl(lease.expires);
-                        if (!server_config.remaining) lease.expires -= time(0);
+                        if (!server_config.remaining)
+                                lease.expires -= time(0);
                         if (!(add_lease(lease.chaddr, lease.yiaddr, lease.expires))) {
                                 bb_error_msg("too many leases while loading %s", file);
                                 break;