interfaces ippp(4) and pppoe(4). Insufficient checking of options presented
by the peer may cause writing of copies of the malicious input beyond the
end of a buffer allocated for that purpose.
Issue found by pavel@
Fix from martin@
This is SA2006-019 (CVE-2006-4304)
Rather than calling mircotime() in catchpacket(), make catchpacket()
take a timeval indicating when the packet was captured. Move
microtime() to the calling functions and grab the timestamp as soon
as we know that we're going to call catchpacket at least once.
This means that we call microtime() once per matched packet, as
opposed to once per matched packet per bpf listener. It also means
that we return the same timestamp to all bpf listeners, rather than
slightly different ones.
It would be more accurate to call microtime() even earlier for all
packets, as you have to grab (1+#listener) locks before you can
determine if the packet will be logged. You could always grab a
timestamp before the locks, but microtime() can be costly, so this
didn't seem like a good idea.
(I guess most ethernet interfaces will have a bpf listener these
days because of dhclient. That means that we could be doing two bpf
locks on most packets going through the interface.)
The former two are no longer necessary as slstats is no more
and pppstats now uses an ioctl instead of rummaging through kmem.
The latter has nothign interesting for the userland, but uses
struct bintime that I'm about to hide under #ifdef _KERNEL.
A bunch of remaining <net/if_*.h> headers is pretty useless to the
userland too, but ... someone else's yag to shave...
- struct timeval time is gone
time.tv_sec -> time_second
- struct timeval mono_time is gone
mono_time.tv_sec -> time_uptime
- access to time via
{get,}{micro,nano,bin}time()
get* versions are fast but less precise
- support NTP nanokernel implementation (NTP API 4)
- further reading:
Timecounter Paper: http://phk.freebsd.dk/pubs/timecounter.pdf
NTP Nanokernel: http://www.eecis.udel.edu/~mills/ntp/html/kern.html
/sys/net/if_spppvar.h says:
"Lower layer drivers that are always ready to communicate
(like hardware HDLC) can shortcut pp_up from pp_tls,
and pp_down from pp_tlf."
When I follow those instructions, I get a kernel stack
overflow as soon as I open the HDLC device.
Here is the loop:
sppp_ioctl calls sppp_lcp_open
sppp_lcp_open calls sppp_open_event
sppp_open_event calls sppp_lcp_tls
sppp_lcp_tls calls pp_tls
pp_tls is the SHORTCUT to sppp_lcp_up
sppp_lcp_up calls spp_lcp_open
...and around we go until the stack overflows.
The fix is to reverse the order of the action (tls)
and the state change (from INITIAL to STARTING) in
sppp_open_event.
There is a similar loop during closing:
sppp_ioctl calls sppp_lcp_close
sppp_lcp_close calls sppp_close_event
spp_close_event calls sppp_lcp_tlf
sppp_lcp_tlf calls pp_tlf
pp_tlf is the SHORTCUT to sppp_lcp_down
sppp_lcp_down calls sppp_lcp_close
...and around we go until the stack overflows.
The fix is to reverse the order of the action (tlf)
and the state change (from STARTING to INITIAL) in
sppp_close_event.
Separately, while I was discovering this, I noticed
that pp_tlf was being called unconditionally rather
than first checking to see if it is NULL. pp_tlf
is a callout from sppp to the hdlc device driver.
Elsewhere in sppp, this is always checked for NULL
before calling it, and the comments in if_spppvar.h
imply that filling it in is optional.
From spppvar.h:
"These functions need to be filled in by the lower layer
(hardware) drivers if they request notification from the
PPP layer whether the link is actually required."
This clearly says that pp_tlf and pp_tls are optional
and so sppp must check before calling them.
that expect real addresses. explicitly KASSERT() that it is not
NULL in the kernel and just avoid using it userland.
(the kernel could be more defensive about this, but, until now it
would have just crashed anyway.)
adrianp