[BusyBox 0003304]: udhcpd arpping overflow bug

[bugs at busybox.net]

The following issue has been CLOSED 
====================================================================== 
http://busybox.net/bugs/view.php?id=3304 
====================================================================== 
Reported By:                Linkn
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   3304
Category:                   Networking Support
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     closed
Resolution:                 open
Fixed in Version:           
====================================================================== 
Date Submitted:             05-09-2008 04:10 PDT
Last Modified:              05-12-2008 17:41 PDT
====================================================================== 
Summary:                    udhcpd arpping overflow  bug
Description: 
In the arpping function(networking\udhcp\arpping.c. line 89),  
prevTime may bring timeout_ms overflow in MIPS os.
Suggest define preTime: unsigned long long preTime;

====================================================================== 

---------------------------------------------------------------------- 
 vda - 05-09-08 04:51  
---------------------------------------------------------------------- 
The code:

        /* wait for arp reply, and check it */
        timeout_ms = 2000;
        do {
                int r;
                unsigned prevTime = monotonic_us();
...
                timeout_ms -= (monotonic_us() - prevTime) / 1000;
        } while (timeout_ms > 0);

Can you explain in more details when overflow occurs? 

---------------------------------------------------------------------- 
 vda - 05-09-08 04:56  
---------------------------------------------------------------------- 
If this is a real bug, it is likely fixed in 21958. 

---------------------------------------------------------------------- 
 Linkn - 05-09-08 20:36  
---------------------------------------------------------------------- 
This is embedded MIPS 4k OS and gcc is mipsel-linux-gcc (GCC) 3.3.6. 
The test code :
	/* wait for arp reply, and check it */
	do {
		int r;
		//unsigned long long prevTime = monotonic_us();
		unsigned  prevTime = monotonic_us();

bb_error_msg("arpping -> timeout_ms:%d\n",timeout_ms);
		pfd[0].events = POLLIN;
		r = safe_poll(pfd, 1, timeout_ms);
		if (r < 0) {
			break;
		} else if (r) {
			if (read(s, &arp, sizeof(arp)) < 0)
				break;
			if (arp.operation == htons(ARPOP_REPLY)
			 && memcmp(arp.tHaddr, from_mac, 6) == 0
			 && *((uint32_t *) arp.sInaddr) == test_ip
			) {
				rv = 0;
				break;
			}
		}
		timeout_ms -= (monotonic_us() - prevTime) / 1000;
	} while (timeout_ms > 0);

Where dhcpd receive discover, debug msg show :

udhcpd: arpping -> timeout_ms:2000
udhcpd: arpping -> timeout_ms:1662154294

At this time,safe_poll will delay long long time .

 

---------------------------------------------------------------------- 
 vda - 05-10-08 08:49  
---------------------------------------------------------------------- 
Please test whether it is fixed by adding (unsigned) cast:

timeout_ms -= ((unsigned)monotonic_us() - prevTime) / 1000; 

---------------------------------------------------------------------- 
 Linkn - 05-11-08 20:44  
---------------------------------------------------------------------- 
Yes ,it is fixed by this mode:

1:  timeout_ms -= ((unsigned)monotonic_us() - prevTime) / 1000; 

2:  unsigned long long prevTime = monotonic_us();

 

---------------------------------------------------------------------- 
 vda - 05-12-08 07:14  
---------------------------------------------------------------------- 
> Yes ,it is fixed by this mode:
> 1: timeout_ms -= ((unsigned)monotonic_us() - prevTime) / 1000;
> 2: unsigned long long prevTime = monotonic_us();

I don't understand. I asked whether _only_ adding (unsigned) is enough.
I'd like to avoid using "long long" as it results in bigger code.

Can you test whether only adding (unsigned) works? 

---------------------------------------------------------------------- 
 Linkn - 05-12-08 07:55  
---------------------------------------------------------------------- 
Yes, I tested. only adding unsigned, it works ok. 

---------------------------------------------------------------------- 
 vda - 05-12-08 17:41  
---------------------------------------------------------------------- 
Fixed in rev 21958. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
05-09-08 04:10  Linkn          New Issue                                    
05-09-08 04:10  Linkn          Status                   new => assigned     
05-09-08 04:10  Linkn          Assigned To               => BusyBox         
05-09-08 04:51  vda            Note Added: 0007584                          
05-09-08 04:56  vda            Note Added: 0007594                          
05-09-08 20:09  Linkn          Issue Monitored: Linkn                       
05-09-08 20:32  Linkn          Note Added: 0007614                          
05-09-08 20:36  Linkn          Note Edited: 0007614                         
05-10-08 08:49  vda            Note Added: 0007624                          
05-11-08 20:43  Linkn          Note Added: 0007634                          
05-11-08 20:44  Linkn          Note Edited: 0007634                         
05-12-08 07:14  vda            Note Added: 0007644                          
05-12-08 07:55  Linkn          Note Added: 0007654                          
05-12-08 17:41  vda            Status                   assigned => closed  
05-12-08 17:41  vda            Note Added: 0007664                          
======================================================================