On 9/18/2009 7:49 AM, Gary Thomas wrote:
On 09/17/2009 05:08 PM, Jay Foster wrote:
The attached patch fixes the DHCP option parsing to deal with DHCP
servers that don't terminate their options with an END tag. The eCos
implementation relies heavily on this. RFC 2132 doesn't specify if this
END tag is required or not. Up until now, I have never seen a DHCP
server do this, but have run into now.
There are other ways to implement this, but I chose this approach as the
least invasive.
How about abstracting the 'recvfrom()+test+repair' sequence
to a function, since there are _four_ identical occurrences?
Attached is a revised patch.
Jay
Index: ecos/packages/net/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/ChangeLog,v
retrieving revision 1.86
diff -u -5 -p -r1.86 ChangeLog
--- ecos/packages/net/common/current/ChangeLog 3 Jul 2009 12:40:25 -0000
1.86
+++ ecos/packages/net/common/current/ChangeLog 22 Sep 2009 16:21:10 -0000
@@ -1,5 +1,10 @@
+2009-09-17 Jay Foster <[email protected]>
+
+ * src/dhcp_prot.c (do_dhcp): Fix packet parsing for DHCP servers
+ that do not terminate the options with an END tag.
+
2009-06-23 Rene Schipp von Branitz Nielsen <[email protected]>
* src/ifaddrs.c (getifaddrs): If socket() call for IPv6 fails,
a stray pointer was freed.
Index: ecos/packages/net/common/current/src/dhcp_prot.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/dhcp_prot.c,v
retrieving revision 1.21
diff -u -5 -p -r1.21 dhcp_prot.c
--- ecos/packages/net/common/current/src/dhcp_prot.c 29 Jan 2009 17:49:57
-0000 1.21
+++ ecos/packages/net/common/current/src/dhcp_prot.c 22 Sep 2009 16:21:11
-0000
@@ -652,20 +652,48 @@ static void set_default_dhcp_tags( struc
// Explicitly specify our max message size.
set_fixed_tag( xmit, TAG_DHCP_MAX_MSGSZ, BP_MINPKTSZ, 2 );
}
// ------------------------------------------------------------------------
+// Get BOOTP/DHCP response.
+// Wait up to the amount of time specified by *tvp.
+
+static int
+get_response(int s, struct bootp *response, struct sockaddr_in *from, struct
timeval *tvp)
+{
+ int pktlen;
+ socklen_t addrlen;
+
+ setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, tvp, sizeof(*tvp));
+
+ addrlen = sizeof(*from);
+ pktlen = recvfrom(s, response, sizeof(*response), 0, (struct sockaddr
*)from, &addrlen);
+ /* Some DHCP servers don't terminate the options list with
+ * an END tag. Append one if we can.
+ */
+ if ((pktlen >= 0) && (pktlen < sizeof(*response)))
+ {
+ /* Do not count the added END tag in the returned packet length.
+ * The returned packet length is the number of bytes received
+ * from the DHCP server. The added END tag is only used
+ * internally for packet parsing purposes.
+ */
+ ((unsigned char *)response)[pktlen] = TAG_END;
+ }
+ return pktlen;
+}
+
+// ------------------------------------------------------------------------
// the DHCP state machine - this does all the work
int
do_dhcp(const char *intf, struct bootp *res,
cyg_uint8 *pstate, struct dhcp_lease *lease)
{
struct ifreq ifr;
struct sockaddr_in cli_addr, broadcast_addr, server_addr, rx_addr;
int s = -1;
- socklen_t addrlen;
int one = 1;
unsigned char mincookie[] = {99,130,83,99,255} ;
struct timeval tv;
struct timeout_state timeout_scratch;
cyg_uint8 oldstate = *pstate;
@@ -842,15 +870,11 @@ do_dhcp(const char *intf, struct bootp *
// This is a separate state so that we can listen again
// *without* retransmitting.
// listen for the DHCPOFFER reply
- setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
-
- addrlen = sizeof(rx_addr);
- if (recvfrom(s, received, sizeof(struct bootp), 0,
- (struct sockaddr *)&rx_addr, &addrlen) < 0) {
+ if (get_response(s, received, &rx_addr, &tv) < 0) {
// No packet arrived (this time)
if ( seen_bootp_reply ) { // then already have a bootp reply
// Save the good packet in *xmit
bcopy( received, xmit, dhcp_size(received) );
*pstate = DHCPSTATE_BOOTP_FALLBACK;
@@ -951,15 +975,11 @@ do_dhcp(const char *intf, struct bootp *
case DHCPSTATE_REQUEST_RECV:
// wait for an ACK or a NACK - retry by going back to
// DHCPSTATE_REQUESTING; NACK means go back to INIT.
- setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
-
- addrlen = sizeof(rx_addr);
- if (recvfrom(s, received, sizeof(struct bootp), 0,
- (struct sockaddr *)&rx_addr, &addrlen) < 0) {
+ if (get_response(s, received, &rx_addr, &tv) < 0) {
// No packet arrived
// go to the next larger timeout and re-send:
if ( ! next_timeout( &tv, &timeout_scratch ) ) {
*pstate = DHCPSTATE_FAILED;
break;
@@ -1097,15 +1117,11 @@ do_dhcp(const char *intf, struct bootp *
case DHCPSTATE_RENEW_RECV:
// wait for an ACK or a NACK - retry by going back to
// DHCPSTATE_RENEWING; NACK means go to NOTBOUND.
// No answer means just wait for T2, to broadcast.
- setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
-
- addrlen = sizeof(rx_addr);
- if (recvfrom(s, received, sizeof(struct bootp), 0,
- (struct sockaddr *)&rx_addr, &addrlen) < 0) {
+ if (get_response(s, received, &rx_addr, &tv) < 0) {
// No packet arrived
// go to the next larger timeout and re-send:
if ( ! next_timeout( &tv, &timeout_scratch ) ) {
// If we timed out completely, just give up until T2
// expires - retain the lease meanwhile. The normal
@@ -1205,15 +1221,11 @@ do_dhcp(const char *intf, struct bootp *
case DHCPSTATE_REBIND_RECV:
// wait for an ACK or a NACK - retry by going back to
// DHCPSTATE_REBINDING; NACK means go to NOTBOUND.
// No answer means just wait for expiry; we tried!
- setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
-
- addrlen = sizeof(rx_addr);
- if (recvfrom(s, received, sizeof(struct bootp), 0,
- (struct sockaddr *)&rx_addr, &addrlen) < 0) {
+ if (get_response(s, received, &rx_addr, &tv) < 0) {
// No packet arrived
// go to the next larger timeout and re-send:
if ( ! next_timeout( &tv, &timeout_scratch ) ) {
// If we timed out completely, just give up until EX
// expires - retain the lease meanwhile. The normal
