[BusyBox 0003304]: udhcpd arpping overflow bug

[bugs at busybox.net]

A NOTE has been added to this issue. 
====================================================================== 
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:                     assigned
====================================================================== 
Date Submitted:             05-09-2008 04:10 PDT
Last Modified:              05-12-2008 07:14 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? 

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                          
======================================================================