1 files changed, 60 insertions, 71 deletions

diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
index 4e3d8ca5e..331f13a8c 100644
--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -605,7 +605,7 @@ static void init_packet(struct dhcp_packet *packet, char type)
         /* Fill in: op, htype, hlen, cookie fields; message type option: */
         udhcp_init_header(packet, type);
 
-        packet->xid = random_xid();
+        packet->xid = client_data.xid;
 
         client_data.last_secs = monotonic_sec();
         if (client_data.first_secs == 0)
@@ -663,6 +663,24 @@ static void add_client_options(struct dhcp_packet *packet)
         //      ...add (DHCP_VENDOR, "udhcp "BB_VER) opt...
 }
 
+static void add_serverid_and_clientid_options(struct dhcp_packet *packet, uint32_t server)
+{
+        struct option_set *ci;
+
+        udhcp_add_simple_option(packet, DHCP_SERVER_ID, server);
+
+        /* RFC 2131 section 2:
+         * If the client uses a 'client identifier' in one message,
+         * it MUST use that same identifier in all subsequent messages.
+         * section 3.1.6:
+         * If the client used a 'client identifier' when it obtained the lease,
+         * it MUST use the same 'client identifier' in the DHCPRELEASE message.
+         */
+        ci = udhcp_find_option(client_data.options, DHCP_CLIENT_ID);
+        if (ci)
+                udhcp_add_binary_option(packet, ci->data);
+}
+
 /* RFC 2131
  * 4.4.4 Use of broadcast and unicast
  *
@@ -701,17 +719,15 @@ static int bcast_or_ucast(struct dhcp_packet *packet, uint32_t ciaddr, uint32_t
 
 /* Broadcast a DHCP discover packet to the network, with an optionally requested IP */
 /* NOINLINE: limit stack usage in caller */
-static NOINLINE int send_discover(uint32_t xid, uint32_t requested)
+static NOINLINE int send_discover(uint32_t requested)
 {
         struct dhcp_packet packet;
 
         /* Fill in: op, htype, hlen, cookie, chaddr fields,
-         * random xid field (we override it below),
-         * message type option:
+         * xid field, message type option:
          */
         init_packet(&packet, DHCPDISCOVER);
 
-        packet.xid = xid;
         if (requested)
                 udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested);
 
@@ -729,7 +745,7 @@ static NOINLINE int send_discover(uint32_t xid, uint32_t requested)
  * "The client _broadcasts_ a DHCPREQUEST message..."
  */
 /* NOINLINE: limit stack usage in caller */
-static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requested)
+static NOINLINE int send_select(uint32_t server, uint32_t requested)
 {
         struct dhcp_packet packet;
         struct in_addr temp_addr;
@@ -748,18 +764,16 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste
  * include that list in all subsequent messages.
  */
         /* Fill in: op, htype, hlen, cookie, chaddr fields,
-         * random xid field (we override it below),
-         * message type option:
+         * xid field, message type option:
          */
         init_packet(&packet, DHCPREQUEST);
 
-        packet.xid = xid;
         udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested);
 
         udhcp_add_simple_option(&packet, DHCP_SERVER_ID, server);
 
         /* Add options: maxsize,
-         * "param req" option according to -O, and options specified with -x
+         * "param req" option according to -O, options specified with -x
          */
         add_client_options(&packet);
 
@@ -775,7 +789,7 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste
 
 /* Unicast or broadcast a DHCP renew message */
 /* NOINLINE: limit stack usage in caller */
-static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
+static NOINLINE int send_renew(uint32_t server, uint32_t ciaddr)
 {
         struct dhcp_packet packet;
 
@@ -794,16 +808,14 @@ static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
  * replying to the client.
  */
         /* Fill in: op, htype, hlen, cookie, chaddr fields,
-         * random xid field (we override it below),
-         * message type option:
+         * xid field, message type option:
          */
         init_packet(&packet, DHCPREQUEST);
 
-        packet.xid = xid;
         packet.ciaddr = ciaddr;
 
         /* Add options: maxsize,
-         * "param req" option according to -O, and options specified with -x
+         * "param req" option according to -O, options specified with -x
          */
         add_client_options(&packet);
 
@@ -821,7 +833,7 @@ static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
 #if ENABLE_FEATURE_UDHCPC_ARPING
 /* Broadcast a DHCP decline message */
 /* NOINLINE: limit stack usage in caller */
-static NOINLINE int send_decline(/*uint32_t xid,*/ uint32_t server, uint32_t requested)
+static NOINLINE int send_decline(uint32_t server, uint32_t requested)
 {
         struct dhcp_packet packet;
 
@@ -830,20 +842,10 @@ static NOINLINE int send_decline(/*uint32_t xid,*/ uint32_t server, uint32_t req
          */
         init_packet(&packet, DHCPDECLINE);
 
-#if 0
-        /* RFC 2131 says DHCPDECLINE's xid is randomly selected by client,
-         * but in case the server is buggy and wants DHCPDECLINE's xid
-         * to match the xid which started entire handshake,
-         * we use the same xid we used in initial DHCPDISCOVER:
-         */
-        packet.xid = xid;
-#endif
         /* DHCPDECLINE uses "requested ip", not ciaddr, to store offered IP */
         udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested);
 
-        udhcp_add_simple_option(&packet, DHCP_SERVER_ID, server);
-
-//TODO: add client-id opt?
+        add_serverid_and_clientid_options(&packet, server);
 
         bb_simple_info_msg("broadcasting decline");
         return raw_bcast_from_client_data_ifindex(&packet, INADDR_ANY);
@@ -856,7 +858,6 @@ ALWAYS_INLINE /* one caller, help compiler to use this fact */
 int send_release(uint32_t server, uint32_t ciaddr)
 {
         struct dhcp_packet packet;
-        struct option_set *ci;
 
         /* Fill in: op, htype, hlen, cookie, chaddr, random xid fields,
          * message type option:
@@ -866,15 +867,7 @@ int send_release(uint32_t server, uint32_t ciaddr)
         /* DHCPRELEASE uses ciaddr, not "requested ip", to store IP being released */
         packet.ciaddr = ciaddr;
 
-        udhcp_add_simple_option(&packet, DHCP_SERVER_ID, server);
-
-        /* RFC 2131 section 3.1.6:
-         * If the client used a 'client identifier' when it obtained the lease,
-         * it MUST use the same 'client identifier' in the DHCPRELEASE message.
-         */
-        ci = udhcp_find_option(client_data.options, DHCP_CLIENT_ID);
-        if (ci)
-                udhcp_add_binary_option(&packet, ci->data);
+        add_serverid_and_clientid_options(&packet, server);
 
         bb_info_msg("sending %s", "release");
         /* Note: normally we unicast here since "server" is not zero.
@@ -1120,7 +1113,7 @@ static void change_listen_mode(int new_mode)
         /* else LISTEN_NONE: client_data.sockfd stays closed */
 }
 
-static void perform_release(uint32_t server_addr, uint32_t requested_ip)
+static void perform_release(uint32_t server_id, uint32_t requested_ip)
 {
         char buffer[sizeof("255.255.255.255")];
         struct in_addr temp_addr;
@@ -1133,12 +1126,13 @@ static void perform_release(uint32_t server_addr, uint32_t requested_ip)
          || client_data.state == REBINDING
          || client_data.state == RENEW_REQUESTED
         ) {
-                temp_addr.s_addr = server_addr;
+                temp_addr.s_addr = server_id;
                 strcpy(buffer, inet_ntoa(temp_addr));
                 temp_addr.s_addr = requested_ip;
                 bb_info_msg("unicasting a release of %s to %s",
                                 inet_ntoa(temp_addr), buffer);
-                send_release(server_addr, requested_ip); /* unicast */
+                client_data.xid = random_xid(); //TODO: can omit?
+                send_release(server_id, requested_ip); /* unicast */
         }
         bb_simple_info_msg("entering released state");
 /*
@@ -1167,7 +1161,7 @@ static void client_background(void)
 //usage:# define IF_UDHCP_VERBOSE(...)
 //usage:#endif
 //usage:#define udhcpc_trivial_usage
-//usage:       "[-fbq"IF_UDHCP_VERBOSE("v")"RB]"IF_FEATURE_UDHCPC_ARPING(" [-a[MSEC]]")" [-t N] [-T SEC] [-A SEC/-n]\n"
+//usage:       "[-fbq"IF_UDHCP_VERBOSE("v")"RB]"IF_FEATURE_UDHCPC_ARPING(" [-a[MSEC]]")" [-t N] [-T SEC] [-A SEC|-n]\n"
 //usage:       "        [-i IFACE]"IF_FEATURE_UDHCP_PORT(" [-P PORT]")" [-s PROG] [-p PIDFILE]\n"
 //usage:       "        [-oC] [-r IP] [-V VENDOR] [-F NAME] [-x OPT:VAL]... [-O OPT]..."
 //usage:#define udhcpc_full_usage "\n"
@@ -1224,9 +1218,8 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
         int tryagain_timeout = 20;
         int discover_timeout = 3;
         int discover_retries = 3;
-        uint32_t server_addr = server_addr; /* for compiler */
+        uint32_t server_id = server_id; /* for compiler */
         uint32_t requested_ip = 0;
-        uint32_t xid = xid; /* for compiler */
         int packet_num;
         int timeout; /* must be signed */
         int lease_remaining; /* must be signed */
@@ -1291,7 +1284,8 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                 memcpy(p + OPT_DATA + 3, str_F, len); /* do not store NUL byte */
         }
         if (opt & OPT_r)
-                requested_ip = inet_addr(str_r);
+                if (!inet_aton(str_r, (void*)&requested_ip))
+                        bb_show_usage();
 #if ENABLE_FEATURE_UDHCP_PORT
         if (opt & OPT_P) {
                 CLIENT_PORT = xatou16(str_P);
@@ -1451,10 +1445,10 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                 if (!discover_retries || packet_num < discover_retries) {
                                         if (packet_num == 0) {
                                                 change_listen_mode(LISTEN_RAW);
-                                                xid = random_xid();
+                                                client_data.xid = random_xid();
                                         }
                                         /* broadcast */
-                                        send_discover(xid, requested_ip);
+                                        send_discover(requested_ip);
                                         timeout = discover_timeout;
                                         packet_num++;
                                         continue;
@@ -1488,7 +1482,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                         case REQUESTING:
                                 if (packet_num < 3) {
                                         /* send broadcast select packet */
-                                        send_select(xid, server_addr, requested_ip);
+                                        send_select(server_id, requested_ip);
                                         timeout = discover_timeout;
                                         packet_num++;
                                         continue;
@@ -1519,7 +1513,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                          * Anyway, it does recover by eventually failing through
                          * into INIT_SELECTING state.
                          */
-                                        if (send_renew(xid, server_addr, requested_ip) >= 0) {
+                                        if (send_renew(server_id, requested_ip) >= 0) {
                                                 timeout = discover_timeout;
                                                 packet_num++;
                                                 continue;
@@ -1548,7 +1542,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                  * try to find DHCP server using broadcast */
                                 if (lease_remaining > 0 && packet_num < 3) {
                                         /* send a broadcast renew request */
-                                        send_renew(xid, 0 /*INADDR_ANY*/, requested_ip);
+                                        send_renew(0 /*INADDR_ANY*/, requested_ip);
                                         timeout = discover_timeout;
                                         packet_num++;
                                         continue;
@@ -1610,7 +1604,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                         timeout = 0;
                         continue;
                 case SIGUSR2:
-                        perform_release(server_addr, requested_ip);
+                        perform_release(server_id, requested_ip);
                         /* ^^^ switches to LISTEN_NONE */
                         timeout = INT_MAX;
                         continue;
@@ -1641,9 +1635,9 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                 continue;
                 }
 
-                if (packet.xid != xid) {
+                if (packet.xid != client_data.xid) {
                         log1("xid %x (our is %x)%s",
-                                (unsigned)packet.xid, (unsigned)xid,
+                                (unsigned)packet.xid, (unsigned)client_data.xid,
                                 ", ignoring packet"
                         );
                         continue;
@@ -1695,13 +1689,13 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
  * They either supply DHCP_SERVER_ID of 0.0.0.0 or don't supply it at all.
  * They say ISC DHCP client supports this case.
  */
-                                server_addr = 0;
+                                server_id = 0;
                                 temp = udhcp_get_option32(&packet, DHCP_SERVER_ID);
                                 if (!temp) {
                                         bb_simple_info_msg("no server ID, using 0.0.0.0");
                                 } else {
                                         /* it IS unaligned sometimes, don't "optimize" */
-                                        move_from_unaligned32(server_addr, temp);
+                                        move_from_unaligned32(server_id, temp);
                                 }
                                 /*xid = packet.xid; - already is */
                                 temp_addr.s_addr = requested_ip = packet.yiaddr;
@@ -1725,7 +1719,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
 
                                 change_listen_mode(LISTEN_NONE);
 
-                                temp_addr.s_addr = server_addr;
+                                temp_addr.s_addr = server_id;
                                 strcpy(server_str, inet_ntoa(temp_addr));
                                 temp_addr.s_addr = packet.yiaddr;
 
@@ -1772,7 +1766,8 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                         ) {
                                                 bb_simple_info_msg("offered address is in use "
                                                         "(got ARP reply), declining");
-                                                send_decline(/*xid,*/ server_addr, packet.yiaddr);
+                                                client_data.xid = random_xid(); //TODO: can omit?
+                                                send_decline(server_id, packet.yiaddr);
 
                                                 if (client_data.state != REQUESTING)
                                                         d4_run_script_deconfig();
@@ -1807,7 +1802,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                 timeout = (unsigned)lease_remaining / 2;
                                 client_data.state = BOUND;
                                 /* make future renew packets use different xid */
-                                /* xid = random_xid(); ...but why bother? */
+                                /* client_data.xid = random_xid(); ...but why bother? */
                                 packet_num = 0;
                                 continue; /* back to main loop */
                         }
@@ -1816,20 +1811,14 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                                  * "wrong" server can reply first, with a NAK.
                                  * Do not interpret it as a NAK from "our" server.
                                  */
-                                if (server_addr != 0) {
-                                        uint32_t svid;
-                                        uint8_t *temp;
-
-                                        temp = udhcp_get_option32(&packet, DHCP_SERVER_ID);
-                                        if (!temp) {
- non_matching_svid:
-                                                log1("received DHCP NAK with wrong"
-                                                        " server ID%s", ", ignoring packet");
-                                                continue;
-                                        }
+                                uint32_t svid = 0; /* we treat no server id as 0.0.0.0 */
+                                uint8_t *temp = udhcp_get_option32(&packet, DHCP_SERVER_ID);
+                                if (temp)
                                         move_from_unaligned32(svid, temp);
-                                        if (svid != server_addr)
-                                                goto non_matching_svid;
+                                if (svid != server_id) {
+                                        log1("received DHCP NAK with wrong"
+                                                " server ID%s", ", ignoring packet");
+                                        continue;
                                 }
                                 /* return to init state */
                                 change_listen_mode(LISTEN_NONE);
@@ -1853,7 +1842,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
 
  ret0:
         if (opt & OPT_R) /* release on quit */
-                perform_release(server_addr, requested_ip);
+                perform_release(server_id, requested_ip);
         retval = 0;
  ret:
         /*if (client_data.pidfile) - remove_pidfile has its own check */