* (no subject)
@ 2005-12-10 17:32 Jody McIntyre
2005-12-10 18:25 ` [PATCH] sbp2: fix panic when ejecting an ipod Stefan Richter
0 siblings, 1 reply; 3+ messages in thread
From: Jody McIntyre @ 2005-12-10 17:32 UTC (permalink / raw)
To: Stefan Richter
Cc: stable, torvalds, linux1394-devel, bcollins, adq, linux-kernel
James Bottomley <James.Bottomley@SteelEye.com>
Bcc:
Subject: Re: [PATCH] sbp2: fix panic when ejecting an ipod
Reply-To:
In-Reply-To: <200512101125.jBABP7Z9001085@einhorn.in-berlin.de>
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.
Cheers,
Jody
>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> Cc: Andrew de Quincey <adq@lidskialf.net>
>
> ---
>
> 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
>
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sbp2: fix panic when ejecting an ipod
2005-12-10 17:32 Jody McIntyre
@ 2005-12-10 18:25 ` Stefan Richter
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Richter @ 2005-12-10 18:25 UTC (permalink / raw)
To: Jody McIntyre
Cc: stable, torvalds, linux1394-devel, bcollins, adq, linux-kernel
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 <stefanr@s5r6.in-berlin.de>
>>Cc: Andrew de Quincey <adq@lidskialf.net>
>>
>>---
>>
>>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/
^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <20051209171922.GW19441@conscoop.ottawa.on.ca>]
* [PATCH] sbp2: fix panic when ejecting an ipod
[not found] <20051209171922.GW19441@conscoop.ottawa.on.ca>
@ 2005-12-10 11:24 ` Stefan Richter
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Richter @ 2005-12-10 11:24 UTC (permalink / raw)
To: stable, torvalds; +Cc: linux1394-devel, bcollins, adq, scjody, linux-kernel
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.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Andrew de Quincey <adq@lidskialf.net>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2005-12-10 18:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-10 17:32 Jody McIntyre
2005-12-10 18:25 ` [PATCH] sbp2: fix panic when ejecting an ipod Stefan Richter
[not found] <20051209171922.GW19441@conscoop.ottawa.on.ca>
2005-12-10 11:24 ` Stefan Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).