Subject: kern/6038: Race condition handling SCSI_NOSLEEP commands
To: None <gnats-bugs@gnats.netbsd.org>
From: Leo Weppelman <leo@x1c19e02.wau.mis.ah.nl>
List: netbsd-bugs
Date: 08/25/1998 15:35:30
>Number:         6038
>Category:       kern
>Synopsis:       Race condition handling SCSI_NOSLEEP commands
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Aug 25 06:50:01 1998
>Last-Modified:
>Originator:     Leo Weppelman
>Organization:
	Just me...
>Release:        NetBSD 1.3G as of Aug. 18, 1998
>Environment:
	
System: NetBSD lwp_hades 1.3G NetBSD 1.3G (HADES++) #1: Tue Aug 18 22:05:15 MEST 1998  root@lwp_hades:/mnt/sys/arch/atari/compile/HADES++ atari


>Description:

As a reference see the thread "Race in scsipi_execute_xs()?" on tech-kern. It
started on 980812.

There is a race condition in the SCSI-code. The race can only happen when
SCSI_NOSLEEP is set.

what can happen is this:
    scsipi_execute_xs()
       scsipi_command_direct() = SUCCESSFULLY_QUEUED
               ..... Some interrupts happen along the way causing the
               scsi-command to have finished in the mean time. NOSLEEP
               is set, so no wakeup/return is done, the xs-structure
               is freed instead (argh!).
    -> now the resuming sections of scsipi_execute_xs() use an already
       freed xs....
>How-To-Repeat:
	
Add a Cabletron SCSI-ethernet box to your NetBSD/Atari config and try to
use it... 

>Fix:
	

A method that works around this bug is given below. It is is snippet of
a mail to tech-kern I wrote. The method works, but I dislike it because
at some point a reference to an already freed structure is made, this
is bad practice...

===== start snippit ====
 thought out a somewhat different approach. It doesn't seem to break
anything, a kernel using it is running now ;-)
What I did is giving each scsipi_xfer structure it's own unique generation
number (extra member: u_int gen_number ). The changes to scsipi_base.c
are globally:
   static int gen_counter;
   
scsipi_get_xs() _and_ scsipi_free_xs()
{
    .....
    xs->gen_number = gen_counter++;
    .....
}

scsipi_execute_xs()
{
     ......
     xs_gen = xs->gen_number;
     switch (scsipi_command_direct(xs)) {
        case SUCCESSFULLY_QUEUED:
              if (xs_gen != xs->gen_number) {
                   /*
                    * Re-used before we got here. Was NOSLEEP case...
                    */
                   return (EJUSTRETURN);
              }
    .......
}

==== end snippit ====
>Audit-Trail:
>Unformatted: