Re: [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference

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

 



On Mon, Mar 10 2008 at 23:50 +0200, Sven Schnelle <svens@xxxxxxxxxxxx> wrote:
> James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
<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@xxxxxxxxxxx>
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@xxxxxxxxxxxx>
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@xxxxxxxxxxx>
---
 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



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux