Bug 28417

Summary: arplookup uses potentially unprotected static variable ...
Product: Base System Reporter: Ernest Hua <ernest>
Component: kernAssignee: Bruce M Simpson <bms>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 4.2-RELEASE   
Hardware: Any   
OS: Any   

Description Ernest Hua 2001-06-26 04:50:02 UTC 
I suspect that there is a hidden problem in using this static variable
in arplookup.  We actually found this while working in VxWorks 5.4, but
it appears to be in FreeBSD and NetBSD as well.  I'll check a few other
OS's later.  The problem is the static variable "sin" in arplookup(),
which is used to hold the IP address to look up.  I am not a network
stack expert, but I suspect this path is not multi-thread friendly, and
will have potentially corrupted results for simultaneous callers.

Fix: 

Remove the "static" keyword.
Comment 1 Bruce M Simpson freebsd_committer freebsd_triage 2003-10-01 10:32:11 UTC 
Responsible Changed
From-To: freebsd-bugs->bms

I've informed sam, who is working on the netperf branch, and given 
him a diff, which I'm testing now.
Comment 2 Bruce M Simpson freebsd_committer freebsd_triage 2003-10-01 14:50:21 UTC 
State Changed
From-To: open->analyzed

A potential race condition exists in the current ARP code. It is 
possible in 5.x UP and SMP kernels that multiple callers could trigger 
a race here on the entry to rtalloc1(), although so far there have not 
been any reported occurrences of this. 

In 4.x there should be sufficient protection through use of splnet(). 
In 5.x this protection does not exist. sam@ is working on fine-grained 
locking for the network stack which should address this issue, and 
a 'starting point' patch is with him as explained above.
Comment 3 Bruce M Simpson freebsd_committer freebsd_triage 2003-10-03 18:35:29 UTC 
State Changed
From-To: analyzed->patched
Comment 4 Bruce M Simpson freebsd_committer freebsd_triage 2003-10-03 18:36:28 UTC 
State Changed
From-To: patched->analyzed

Snafu log message
Comment 5 Bruce M Simpson freebsd_committer freebsd_triage 2003-10-03 18:37:01 UTC 
State Changed
From-To: analyzed->patched

Thanks for telling us about this issue. Sam has committed my fix 
to the netperf tree. I am marking this as patched for the time being; 
we will consider it closed when the final netperf merge into mainline 
development takes place probably in time for 5.2-RELEASE.
Comment 6 Bruce M Simpson freebsd_committer freebsd_triage 2003-11-15 08:42:28 UTC 
State Changed
From-To: patched->closed

Believed that issue would affect the network stack post-locking-patches. 

Not believed that issue would affect 4.x SMP as the RELENG_4 branch 
still uses spl*() and friends to protect the sections involved. 

Something similar to the submitter's patch has been committed and will 
be present in 5.2-RELEASE.