LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Sven Schnelle <svens@bitebene.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: linux-kernel@vger.kernel.org,
	linux-scsi <linux-scsi@vger.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference
Date: Tue, 11 Mar 2008 17:47:43 +0200	[thread overview]
Message-ID: <47D6A99F.4040408@panasas.com> (raw)
In-Reply-To: <86zlt68d0z.fsf@deprecated.bitebene.org>

On Mon, Mar 10 2008 at 23:50 +0200, Sven Schnelle <svens@bitebene.org> wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> 
>> On Mon, 2008-03-10 at 17:20 +0200, Boaz Harrosh wrote:
>>> James and linux-scsi CCed.
>> Looks fine .. could someone send the patch in an applyable form (i.e.
>> not quoted).
> 
> Sure:
> 
> Fix NULL pointer dereference during execution of Internal commands,
> where gdth only allocates scp, but not scp->sense_buffer. The rest of
> the code assumes that sense_buffer is allocated, which leads to a kernel
> oops e.g. on reboot (during cache flush).
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
<snip>

Hi Sven.

Do you have gdth HW for testing patches?
I'm anticipating more scsi-ml changes in that regard in near future and would like
a more permanent solution for gdth. Could you please try below patch in place of
your patch and see if it works.

Thanks in advance

James Hi
do you think we should keep Sven's patch for the rc-fixes and my solution
for the next kernel? For the reason that my patch might be theoretically dangerous 
in regard to locking, queue-life-time, and such side effects?

Boaz
--- 
>From 1795a2063eabf326c4fba49af4db6572bdd642b6 Mon Sep 17 00:00:00 2001
From: Boaz Harrosh <bharrosh@panasas.com>
Date: Tue, 11 Mar 2008 17:42:06 +0200
Subject: [PATCH] gdth: Use scsi_get_command for gdth internal commands

As found by: Sven Schnelle <svens@bitebene.org>
NULL pointer dereference bug during execution of Internal commands,
where gdth only allocates scp, but not scp->sense_buffer. The rest of
the code assumes that sense_buffer is allocated, which leads to a kernel
oops e.g. on reboot (during cache flush).

Fix this by leting scsi-ml allocate the command for us, in anticipation
of future changes to commands allocation.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/gdth.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 4560e39..643e7d6 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -448,7 +448,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
     DECLARE_COMPLETION_ONSTACK(wait);
     int rval;
 
-    scp = kzalloc(sizeof(*scp), GFP_KERNEL);
+    scp = scsi_get_command(sdev, GFP_KERNEL);
     if (!scp)
         return -ENOMEM;
 
@@ -472,7 +472,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
     rval = cmndinfo.status;
     if (info)
         *info = cmndinfo.info;
-    kfree(scp);
+    scsi_put_command(scp);
     return rval;
 }
 
-- 
1.5.3.3




  reply	other threads:[~2008-03-11 15:48 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-09 12:41 Sven Schnelle
2008-03-10 15:20 ` Boaz Harrosh
2008-03-10 21:12   ` James Bottomley
2008-03-10 21:50     ` Sven Schnelle
2008-03-11 15:47       ` Boaz Harrosh [this message]
2008-03-11 16:16     ` Boaz Harrosh
2008-03-11 17:39       ` Matthew Dharm
2008-03-11 18:07       ` Alan Stern
2008-03-11 18:36         ` Boaz Harrosh
2008-03-11 19:18           ` Alan Stern
2008-03-12 13:07             ` Boaz Harrosh
2008-03-12 13:11               ` [PATCH] isd200: Allocate sense_buffer for hacked up scsi_cmnd Boaz Harrosh
2008-03-12 15:10                 ` Alan Stern
2008-03-12 15:24                   ` [PATCH resend] " Boaz Harrosh
2008-03-12 16:54                     ` James Bottomley
2008-03-12 17:05                       ` Boaz Harrosh
2008-03-12 17:20                         ` [PATCH ver3] " Boaz Harrosh
2008-03-13 20:01                           ` Andrew Morton
2008-03-13 20:16                             ` James Bottomley
2008-03-12 13:55               ` [PATCH] isd200: Fix memory leak in isd200_get_inquiry_data Boaz Harrosh
2008-03-12 15:11                 ` Alan Stern
2008-03-12 15:08               ` [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47D6A99F.4040408@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=svens@bitebene.org \
    --subject='Re: [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).