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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47D6A99F.4040408@panasas.com>
Date:	Tue, 11 Mar 2008 17:47:43 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Sven Schnelle <svens@...ebene.org>,
	James Bottomley <James.Bottomley@...senPartnership.com>
CC:	linux-kernel@...r.kernel.org,
	linux-scsi <linux-scsi@...r.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
Subject: Re: [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer
 dereference

On Mon, Mar 10 2008 at 23:50 +0200, Sven Schnelle <svens@...ebene.org> wrote:
> James Bottomley <James.Bottomley@...senPartnership.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@...ckframe.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@...asas.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@...ebene.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@...asas.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



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/