Subject: kern/9928: i82365_isasubr.c vs. isa_intr_disestablish
To: None <gnats-bugs@gnats.netbsd.org>
From: Bill Sommerfeld <sommerfeld@netbsd.org>
List: netbsd-bugs
Date: 04/18/2000 19:53:14
>Number:         9928
>Category:       kern
>Synopsis:       i82365_isasubr.c vs. isa_intr_disestablish
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 18 19:54:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Bill Sommerfeld
>Release:        20000416
>Organization:
	none.
>Environment:
	
System: NetBSD thunk 1.4X NetBSD 1.4X (VAIO) #4: Tue Mar 28 11:00:01 CST 2000 wes@thunk:/usr/wes/sys-current/arch/i386/compile/VAIO i386


>Description:
	currently, pcic_isa_count_intr() in i82365_isasubr.c calls
	isa_intr_disestablish() at interrupt level, while the
	interrupt in question is being handled.
	isa_intr_disestablish() frees the "struct intrhand", and then
	the stub code in vector.s dereferences the next pointer in the freed
	intrhand, resulting in immediate carnage -- most likely, branching
	to random data followed by a machine reset -- since the
	ih_next pointer was likely overwritten by the free() in 
	isa_intr_disestablish().
>How-To-Repeat:
	???.  questionable code noticed by Enami-san.
	This may be a problem which prevents the -20000416
	-current snapshot from successfully booting on a Sony Vaio C1
	(i have second-hand reports at this time; I hope to have
	access to the machine in question in a day or two..)
>Fix:
	Probably the simplest fix would be to fetch the
	"ih_next" pointer before calling the handler, so that the
	intrhand is not referenced after the handler returns.

speculative fix (untested) for i386; most other ports will need
similar fixes:

RCS file: /cvsroot/syssrc/sys/arch/i386/isa/vector.s,v
retrieving revision 1.46
diff -u -r1.46 vector.s
--- vector.s    2000/01/20 03:19:27     1.46
+++ vector.s    2000/04/19 02:46:29
@@ -202,11 +202,12 @@
        jnz     4f                                                      ;\
        movl    %esp,%eax               /* 0 means frame pointer */     ;\
 4:     pushl   %eax                                                    ;\
-       call    IH_FUN(%ebx)            /* call it */                   ;\
-       addl    $4,%esp                 /* toss the arg */              ;\
-       STRAY_INTEGRATE                 /* maybe he claimed it */       ;\
+       movl    IH_FUN(%ebx),%eax                                       ;\
        incl    IH_COUNT(%ebx)          /* count the intrs */           ;\
        movl    IH_NEXT(%ebx),%ebx      /* next handler in chain */     ;\
+       call    %eax                    /* call it */                   ;\
+       addl    $4,%esp                 /* toss the arg */              ;\
+       STRAY_INTEGRATE                 /* maybe he claimed it */       ;\
        testl   %ebx,%ebx                                               ;\
        jnz     7b                                                      ;\
        STRAY_TEST                      /* see if it's a stray */       ;\


>Release-Note:
>Audit-Trail:
>Unformatted: