[9fans] off by one in the pc kaddr

[9fans] off by one in the pc kaddr

[Anthony Martin]

I've read through the MMU code more than
a few times and never noticed this.  Who
reads past tmpunmap anyways? ;)

  Anthony

diff -c /sys/src/9/pc/mmu.c /tmp/mmu.c
/sys/src/9/pc/mmu.c:934,940 - /tmp/mmu.c:934,940
  void*
  kaddr(ulong pa)
  {
- 	if(pa > (ulong)-KZERO)
+ 	if(pa >= (ulong)-KZERO)
  		panic("kaddr: pa=%#.8lux", pa);
  	return (void*)(pa+KZERO);
  }

[9fans] off by one in the pc kaddr

[erik quanstrom]

On Fri Mar 18 06:32:08 EDT 2011, ality at pbrane.org wrote:
> I've read through the MMU code more than
> a few times and never noticed this.  Who
> reads past tmpunmap anyways? ;)
> 
>   Anthony
> 
> diff -c /sys/src/9/pc/mmu.c /tmp/mmu.c
> /sys/src/9/pc/mmu.c:934,940 - /tmp/mmu.c:934,940
>   void*
>   kaddr(ulong pa)
>   {
> - 	if(pa > (ulong)-KZERO)
> + 	if(pa >= (ulong)-KZERO)
>   		panic("kaddr: pa=%#.8lux", pa);
>   	return (void*)(pa+KZERO);
>   }

of course, this would allow one to KADDR(-KZERO).
and get 0.  good call.  cf.  cankaddr().

unfortunately, xalloc() is not playing along.  if you have
a kernel that's using a full -KZERO, then your kernel
will panic in xinit() when it tries to set m->klimit to
KADDR(m->base+n*BY2PG), since m->base+n*BY2PG =
-KZERO.

perhaps the correct fix is to store the Confmem
range as [a,b] not [a, b). so

../port/devproc.c:754: 			if(cm->kbase <= offset && offset <= cm->klimit-1){
../port/devproc.c:755: 				if(offset+n >= cm->klimit-1)
../port/devproc.c:756: 					n = cm->klimit - offset;
../port/xalloc.c:75: 				m->klimit = (ulong)KADDR(m->base+n*BY2PG);

would be

../port/devproc.c:753: 			if(cm->kbase <= offset && offset <= cm->klimit){
../port/devproc.c:754: 				if(offset+n >= cm->klimit)
../port/devproc.c:755: 					n = cm->klimit - offset + 1;
../port/xalloc.c:72: 			m->klimit = (ulong)KADDR(m->base+n*BY2PG-1);

there's even a comment you can remove
			/* klimit-1 because klimit might be zero! */

- erik