ide-scsi ref count bug & oops in 2.4.20-ac2/2.4.21-pre2

This bug affects modular ide-scsi in 2.4.20-ac2 and 2.4.21-pre2. Inserting the ide-scsi module and letting it control an odd number of drives, and then rmmod:ing ide-scsi, results in the following:

(after inserting ide-scsi; hdc is my CD writer) scsi0 : SCSI host adapter emulation for IDE ATAPI devices
Vendor: PHILIPS Model: CDRW2412A Rev: P1.5
Type: CD-ROM ANSI SCSI revision: 02

(after rmmod ide-scsi)
scsi : 0 hosts left.
idescsi_cleanup: hdc: failed to unregister!
hdc: usage 0, busy 0, driver d08a00e0, Dbusy 1
hdc: exit_idescsi_module() called while still busy

Rebooting after this then triggers an oops in ide.c:ide_notify_reboot()'s call to DRIVER(drive)->cleanup(drive), since DRIVER(drive) [hdc] still points to the idescsi_driver structure in the unloaded ide-scsi module.

It turns out that there is a long-standing reference counting bug in all versions of ide-scsi in current 2.2/2.4/2.5 kernels: ide-scsi abuses DRIVER(drive)->busy as an integer counter when in fact ->busy is only a 1-bit field.

In 2.4.20-ac2/2.4.21-pre2, idescsi_driver.busy is zero after init because idescsi_setup() does one ++ and one -- for each drive. At "rmmod ide-scsi", idescsi_release() does a DRIVER(drive)->busy-- on each ide-scsi drive. Since they all point to the same idescsi_driver, idescsi_driver.busy is decremented to -(number of drives) % 2. With an odd number of ide-scsi drives, the driver is marked busy, idescsi_cleanup() fails to clean up, and ide_notify_reboot() oopses.

ide-scsi.c in other kernels (2.2, 2.4 before 20-ac2/21-pre2, 2.5) also abuses DRIVER(drive)->busy, but there it has two complementary bugs that cancel each other. idescsi_setup() increments ->busy for each ide-scsi drive, and idescsi_release() decrements ->busy for each drive. busy equals (number of drives) % 2 while ide-scsi is loaded, but is zero as the module is being removed. ide-scsi unloads cleanly, but ->busy clearly incorrect and useless.

The patch below applies to 2.4.20-ac2 and 2.4.21-pre2. It's a minimal
fix: instead of decrementing busy, idescsi_release() sets busy to zero since that's the only valid value at this point. Tested & works for me.

Long-term, someone needs to decide whether idescsi_release() really needs to loop over all drives. They all point to idescsi_driver, so a single "idescsi_driver.busy = 0;" should suffice.

/Mikael

@@ -900,7 +900,7 @@
 	for (id = 0; id < MAX_HWIFS * MAX_DRIVES; id++) {
 		drive = idescsi_drives[id];
 		if (drive)
-			DRIVER(drive)->busy--;
+			DRIVER(drive)->busy = 0;
 	}
 	return 0;
 }