Re: [PATCH] sbp2: fix panic when ejecting an ipod

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jody McIntyre wrote:
On Sat, Dec 10, 2005 at 12:24:59PM +0100, Stefan Richter wrote:

sbp2: fix panic when ejecting an ipod

Sbp2 did not catch some bogus transfer directions in requests from upper
layers.  Problem became apparent when iPods were to be ejected:
http://marc.theaimsgroup.com/?l=linux1394-devel&m=113399994920181
http://marc.theaimsgroup.com/?l=linux1394-user&m=112152701817435
Debugging and original variant of the patch by Andrew de Quincey.


NAK.  James has a patch to fix this in the SCSI layer, which is his
preference.

The fix for SCSI mid/high layer is to send the proper transfer direction. (BTW, note that there are _many_ places where the transfer direction is generated, not just where Jeff's and James' patches correct things. The code which generates transfer directions will keep changing. Userspace can set transfer directions.)

The fix for sbp2 is to not cause a kernel panic should an improper transfer direction be send. All SCSI low level drivers have to handle the transfer direction one way or another. Most chose not to panic.

My patch is not the fix for the wrong direction. It is solely meant not to crash the whole machine after a minor mistake. Also, my patch is not meant to hide mistakes that occurred in higer levels --- and it doesn't do so.

I absolutely agree with you et al that bugs must be fixed in the layer where they occur. However I also strongly believe that a bug in one layer should not trickle down two or more layers and increase the damage up to a catastrophe --- *if this can be avoided by simple means*, i.e. without bloat. (Note the diffstat. I am basically moving existing code up in an if/else cascade. Also, another cleanup for sbp2_create_command_orb will follow next week.)

Please apply.

Signed-off-by: Stefan Richter <[email protected]>
Cc: Andrew de Quincey <[email protected]>

---

Corresponding fix of the places where transfer direction is actually set and
how to clean sbp2_create_command_orb() up after this fix is being discussed.
A patch for SCSI mid and high level has been submitted.
http://marc.theaimsgroup.com/?t=113400010000001
Please apply the following patch to prevent kernel panics as the first step.

drivers/ieee1394/sbp2.c |   16 ++++++----------
1 files changed, 6 insertions(+), 10 deletions(-)

--- linux/drivers/ieee1394.orig/sbp2.c	2005-11-24 23:10:21.000000000 +0100
+++ linux/drivers/ieee1394/sbp2.c	2005-12-10 11:57:41.000000000 +0100
@@ -1784,6 +1784,12 @@ static int sbp2_create_command_orb(struc
			break;
	}

+	if (orb_direction != ORB_DIRECTION_NO_DATA_TRANSFER &&
+	    scsi_request_bufflen == 0) {
+		SBP2_WARN("Enforcing transfer direction DMA_NONE");
+		orb_direction = ORB_DIRECTION_NO_DATA_TRANSFER;
+	}
+
	/*
	 * Set-up our pagetable stuff... unfortunately, this has become
	 * messier than I'd like. Need to clean this up a bit.   ;-)
@@ -1900,16 +1906,6 @@ static int sbp2_create_command_orb(struc
			command_orb->misc |= ORB_SET_DATA_SIZE(scsi_request_bufflen);
			command_orb->misc |= ORB_SET_DIRECTION(orb_direction);

-			/*
-			 * Sanity, in case our direction table is not
-			 * up-to-date
-			 */
-			if (!scsi_request_bufflen) {
-				command_orb->data_descriptor_hi = 0x0;
-				command_orb->data_descriptor_lo = 0x0;
-				command_orb->misc |= ORB_SET_DIRECTION(1);
-			}
-
		} else {
			/*
			 * Need to turn this into page tables, since the





--
Stefan Richter
-=====-=-=-= ==-- -=-=-
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux