[patch 29/29] bug in gdth.c crashing machine

[patch 29/29] bug in gdth.c crashing machine

[akpm]

From: Joerg Dorchain <joerg@dorchain.net>

Undocumented...

Signed-off-by: Joerg Dorchain <joerg@dorchain.net>
Acked-by: "Leubner, Achim" <Achim_Leubner@adaptec.com>
Cc: <stable@kernel.org>
Cc: James Bottomley <James.Bottomley@steeleye.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/scsi/gdth.c |    2 ++
 1 file changed, 2 insertions(+)

diff -puN drivers/scsi/gdth.c~bug-in-gdthc-crashing-machine drivers/scsi/gdth.c
--- a/drivers/scsi/gdth.c~bug-in-gdthc-crashing-machine
+++ a/drivers/scsi/gdth.c
@@ -3091,6 +3091,7 @@ static int gdth_fill_raw_cmd(int hanum,S
             cmdp->u.raw64.direction  = 
                 gdth_direction_tab[scp->cmnd[0]]==DOU ? GDTH_DATA_OUT:GDTH_DATA_IN;
             memcpy(cmdp->u.raw64.cmd,scp->cmnd,16);
+            cmdp->u.raw64.sg_ranz    = 0;
         } else {
             cmdp->u.raw.reserved   = 0;
             cmdp->u.raw.mdisc_time = 0;
@@ -3107,6 +3108,7 @@ static int gdth_fill_raw_cmd(int hanum,S
             cmdp->u.raw.direction  = 
                 gdth_direction_tab[scp->cmnd[0]]==DOU ? GDTH_DATA_OUT:GDTH_DATA_IN;
             memcpy(cmdp->u.raw.cmd,scp->cmnd,12);
+            cmdp->u.raw.sg_ranz    = 0;
         }
 
         if (scp->use_sg) {
_

Re: [patch 29/29] bug in gdth.c crashing machine

[James Bottomley]

On Tue, 2007-03-06 at 02:46 -0800, akpm@linux-foundation.org wrote:
> From: Joerg Dorchain <joerg@dorchain.net>
> 
> Undocumented...

This is a bit of a problem, isn't it?

I have no report of any problem this fixes, and no input from the gdth
maintainer about this.  The first I actually knew of this patch request
is that it appeared in an akpm patch stream yesterday.

I really think we need more details before this gets applied to
anything, let alone the stable tree.

James

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[Greg KH]

On Wed, Mar 07, 2007 at 11:28:25AM -0600, James Bottomley wrote:
> On Tue, 2007-03-06 at 02:46 -0800, akpm@linux-foundation.org wrote:
> > From: Joerg Dorchain <joerg@dorchain.net>
> > 
> > Undocumented...
> 
> This is a bit of a problem, isn't it?
> 
> I have no report of any problem this fixes, and no input from the gdth
> maintainer about this.  The first I actually knew of this patch request
> is that it appeared in an akpm patch stream yesterday.
> 
> I really think we need more details before this gets applied to
> anything, let alone the stable tree.

Randy did forward it to the stable team a while ago too, but I didn't
know even what tree it applied to as the patch level was a bit off.  I
think he might have some more information about it?

thanks,

greg k-h

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[Andrew Morton]

On Wed, 7 Mar 2007 09:40:21 -0800 Greg KH <greg@kroah.com> wrote:

> On Wed, Mar 07, 2007 at 11:28:25AM -0600, James Bottomley wrote:
> > On Tue, 2007-03-06 at 02:46 -0800, akpm@linux-foundation.org wrote:
> > > From: Joerg Dorchain <joerg@dorchain.net>
> > > 
> > > Undocumented...
> > 
> > This is a bit of a problem, isn't it?
> > 
> > I have no report of any problem this fixes, and no input from the gdth
> > maintainer about this.  The first I actually knew of this patch request
> > is that it appeared in an akpm patch stream yesterday.
> > 
> > I really think we need more details before this gets applied to
> > anything, let alone the stable tree.
> 
> Randy did forward it to the stable team a while ago too, but I didn't
> know even what tree it applied to as the patch level was a bit off.  I
> think he might have some more information about it?
> 

Achim did reply:  http://lkml.org/lkml/2007/2/23/138

So we don't know what the patch does, but it should be merged into -stable
(and mainline, heaven forfend)

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[Joerg Dorchain]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Wed, Mar 07, 2007 at 11:16:16AM -0800, Andrew Morton wrote:
> > On Wed, Mar 07, 2007 at 11:28:25AM -0600, James Bottomley wrote:
> > > On Tue, 2007-03-06 at 02:46 -0800, akpm@linux-foundation.org wrote:
> > > > From: Joerg Dorchain <joerg@dorchain.net>
> > > > 
> > > > Undocumented...
> > > 
> > > This is a bit of a problem, isn't it?
> > > 
> > > I have no report of any problem this fixes, and no input from the gdth
> > > maintainer about this.  The first I actually knew of this patch request
> > > is that it appeared in an akpm patch stream yesterday.
> > > 
> > > I really think we need more details before this gets applied to
> > > anything, let alone the stable tree.
> > 
> > Randy did forward it to the stable team a while ago too, but I didn't
> > know even what tree it applied to as the patch level was a bit off.  I
> > think he might have some more information about it?
> > 
> 
> Achim did reply:  http://lkml.org/lkml/2007/2/23/138

The current gdth driver crashes the machine with a panic() when
accessing certain tape drives, really not a good thing for stable.

This patch fixes it, tested by me and acknowledged by Achim.
See http://lkml.org/lkml/2006/10/13/60 and
http://lkml.org/lkml/2006/10/12/35 for further reference.

The patch is against Linus' 2.6.20

Bye,

Joerg

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[James Bottomley]

On Wed, 2007-03-07 at 11:16 -0800, Andrew Morton wrote:
> Achim did reply:  http://lkml.org/lkml/2007/2/23/138

Ah ... OK; sorry, I'm parochial ... if it didn't appear on linux-scsi,
you can usually assume I haven't seen it.

> So we don't know what the patch does, but it should be merged into
> -stable
> (and mainline, heaven forfend)

Well ... I would, except what this patch does is to initialise the
command sg_ranz field (for both 64 bit and 32 bit commands).  If you
look in the code just below the patch application at the 

if (scp->use_sg) {
...
} else if (scp->request_bufflen) {
...
}

You'll find a line setting these parameters in each of the cases of the
if statement.  So the bug appears to be that there's a missing else
clause to this if, which would initialise the zero transfer commands.
Is that a correct analysis?

James

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[Greg KH]

On Wed, Mar 07, 2007 at 07:50:14PM -0500, James Bottomley wrote:
> On Wed, 2007-03-07 at 11:16 -0800, Andrew Morton wrote:
> > Achim did reply:  http://lkml.org/lkml/2007/2/23/138
> 
> Ah ... OK; sorry, I'm parochial ... if it didn't appear on linux-scsi,
> you can usually assume I haven't seen it.
> 
> > So we don't know what the patch does, but it should be merged into
> > -stable
> > (and mainline, heaven forfend)
> 
> Well ... I would, except what this patch does is to initialise the
> command sg_ranz field (for both 64 bit and 32 bit commands).  If you
> look in the code just below the patch application at the 
> 
> if (scp->use_sg) {
> ...
> } else if (scp->request_bufflen) {
> ...
> }
> 
> You'll find a line setting these parameters in each of the cases of the
> if statement.  So the bug appears to be that there's a missing else
> clause to this if, which would initialise the zero transfer commands.
> Is that a correct analysis?

Well, as no one has really responded to this thread about what the patch
really does, and it's not in even James's tree yet, I've dropped it from
-stable.

If James does send it to Linus, I'll be glad to reconsider it at that
time.

thanks,

greg k-h

Re: [stable] [patch 29/29] bug in gdth.c crashing machine

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On Fri, 2007-03-09 at 10:36 -0800, Greg KH wrote:
> On Wed, Mar 07, 2007 at 07:50:14PM -0500, James Bottomley wrote:
> > On Wed, 2007-03-07 at 11:16 -0800, Andrew Morton wrote:
> > > Achim did reply:  http://lkml.org/lkml/2007/2/23/138
> > 
> > Ah ... OK; sorry, I'm parochial ... if it didn't appear on linux-scsi,
> > you can usually assume I haven't seen it.
> > 
> > > So we don't know what the patch does, but it should be merged into
> > > -stable
> > > (and mainline, heaven forfend)
> > 
> > Well ... I would, except what this patch does is to initialise the
> > command sg_ranz field (for both 64 bit and 32 bit commands).  If you
> > look in the code just below the patch application at the 
> > 
> > if (scp->use_sg) {
> > ...
> > } else if (scp->request_bufflen) {
> > ...
> > }
> > 
> > You'll find a line setting these parameters in each of the cases of the
> > if statement.  So the bug appears to be that there's a missing else
> > clause to this if, which would initialise the zero transfer commands.
> > Is that a correct analysis?
> 
> Well, as no one has really responded to this thread about what the patch
> really does, and it's not in even James's tree yet, I've dropped it from
> -stable.
> 
> If James does send it to Linus, I'll be glad to reconsider it at that
> time.

I found the correct explanation.  It was actually given by the bug
reporter a while ago.  I've got the following in scsi-rc-fixes-2.6,
which should be upstream track.

James



[-- Attachment #2: Type: application/mbox, Size: 1993 bytes --]