Subject: kern/27749: sbp2 driver crashes on unsolicited events
To: None <gnats-bugs@gnats.NetBSD.org>
From: None <mlelstv@serpens.de>
List: netbsd-bugs
Date: 11/01/2004 09:59:49
>Number:         27749
>Category:       kern
>Synopsis:       sbp2 driver crashes on unsolicited events
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 01 08:56:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Michael van Elst
>Release:        NetBSD 2.0_RC4
>Organization:
	
>Environment:
	
	
System: NetBSD sagitta 2.0_RC4 NetBSD 2.0_RC4 (SAGITTA) #1: Tue Oct 19 19:53:34 MEST 2004 spz@sagitta:/sys/arch/i386/compile/SAGITTA i386
Architecture: i386
Machine: i386
>Description:
The sbp2 driver can crash the kernel when it sees an unsolicited
response in state SBP2_ORB_STATUS_STATE.

The driver searches for a corresponding status orb:

                        simple_lock(&orb->sbp2->orblist_lock);
                        CIRCLEQ_FOREACH(statorb, &orb->sbp2->orbs, orb_list) {
                                if (addr == statorb->cmd.ab_addr) {
                                        found = 1;
                                        break;
                                }
                        }
                        simple_unlock(&orb->sbp2->orblist_lock);

If no matching orb is found the value of statorb (which points to the
CIRCLEQ head at that time) is still used.

                        simple_lock(&statorb->orb_lock);
                        /* XXX: Need to handle unsolicited correctly. */
                        if (SBP2_STATUS_GET_DEAD(ntohl(abuf->ab_data[0]))) {
                                DPRINTFN(1, ("Transitioning to dead state\n"));
                                statorb->lun->state = SBP2_STATE_DEAD;
                        }

>How-To-Repeat:
Use sbp2 devices with the firewire driver (which may require more changes
to the firewire code).

>Fix:

Index: sbp2.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ieee1394/sbp2.c,v
retrieving revision 1.18.4.1
diff -u -r1.18.4.1 sbp2.c
--- sbp2.c	2 Jul 2004 17:27:56 -0000	1.18.4.1
+++ sbp2.c	1 Nov 2004 08:45:36 -0000
@@ -956,13 +956,6 @@
 				}
 			}
 			simple_unlock(&orb->sbp2->orblist_lock);
-			simple_lock(&statorb->orb_lock);
-
-			/* XXX: Need to handle unsolicited correctly. */
-			if (SBP2_STATUS_GET_DEAD(ntohl(abuf->ab_data[0]))) {
-				DPRINTFN(1, ("Transitioning to dead state\n"));
-				statorb->lun->state = SBP2_STATE_DEAD;
-			}
 			if (!found) {
 #ifdef SBP2_DEBUG
 				u_int32_t i = ntohl(abuf->ab_data[0]);
@@ -975,6 +968,13 @@
 				    SBP2_STATUS_GET_LEN(i) - 1));
 				return;
 			}
+			simple_lock(&statorb->orb_lock);
+
+			/* XXX: Need to handle unsolicited correctly. */
+			if (SBP2_STATUS_GET_DEAD(ntohl(abuf->ab_data[0]))) {
+				DPRINTFN(1, ("Transitioning to dead state\n"));
+				statorb->lun->state = SBP2_STATE_DEAD;
+			}
 
 			/*
 		 	 * After it's been sent turn this into a dummy orb. 

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