On Thu, 2 Dec 2010, Bruce Cran wrote:
Log:
Disallow passing in a count of zero bytes to the bus_space(9) functions.
Passing a count of zero on i386 and amd64 for [I386|AMD64]_BUS_SPACE_MEM
causes a crash/hang since the 'loop' instruction decrements the counter
before checking if it's zero.
PR: kern/80980
Discussed with: jhb
...
Modified: head/sys/amd64/include/bus.h
==============================================================================
--- head/sys/amd64/include/bus.h Thu Dec 2 22:00:57 2010
(r216133)
+++ head/sys/amd64/include/bus.h Thu Dec 2 22:19:30 2010
(r216134)
@@ -104,6 +104,9 @@
#ifndef _AMD64_BUS_H_
#define _AMD64_BUS_H_
+#include <sys/param.h>
+#include <sys/systm.h>
+
This is massive namespace pollution.
Most kernel .c files should include these first, and most already do.
(Ones that try to be smart and only include <sys/types.h> instead of
<sys/param.h>, or <sys/param.h> without <sys/systm.h>, or include
<sys/systm.h> after other headers, may already be broken, since
KASSERT() is declared in <sys/systm.h>, but it may be used in other
header (like this one now). KASSERT() should probably be declared in
<sys/param.h> or even in <sys/cdefs.h>. That gives more pollution there
but less overall.)
#include <machine/_bus.h>
#include <machine/cpufunc.h>
Including <machine/_bus.h> is correct (_bus.h exist to avoid namespace
pollution that is about 1000 times smaller than now here), but including
<machine/cpufunc.h> is older namespace pollution/historical mislayering
(we only need i/o functions from cpufunc.h, and they should be declared
here directly). Now it has no effect, since <machine/cpufunc.h> is
standard namespace pollution in <sys/systm.h>.
@@ -268,7 +271,7 @@ static __inline void
bus_space_read_multi_1(bus_space_tag_t tag, bus_space_handle_t bsh,
bus_size_t offset, u_int8_t *addr, size_t count)
{
-
+ KASSERT(count != 0, ("%s: count == 0", __func__));
if (tag == AMD64_BUS_SPACE_IO)
insb(bsh + offset, addr, count);
else {
KASSERT() in little inline functions gives a lot of bloat for such an
unlikely error. Stupid callers can still pass any garbage count except 0.
The function name of a leaf function is not very interesting. In some
of the other bus.h's, the caller's name is available since the interface
is a macro, but there __func__ (which should only be used in macros)
is not used, apparently since it would give a name that is useful but
inconsistent with arches that don't use a macro.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
