Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751668AbYCJPUw (ORCPT ); Mon, 10 Mar 2008 11:20:52 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750925AbYCJPUo (ORCPT ); Mon, 10 Mar 2008 11:20:44 -0400 Received: from bzq-219-195-70.pop.bezeqint.net ([62.219.195.70]:57385 "EHLO bh-buildlin2.bhalevy.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750874AbYCJPUm (ORCPT ); Mon, 10 Mar 2008 11:20:42 -0400 Message-ID: <47D551B8.9080807@panasas.com> Date: Mon, 10 Mar 2008 17:20:24 +0200 From: Boaz Harrosh User-Agent: Thunderbird 2.0.0.9 (X11/20071031) MIME-Version: 1.0 To: Sven Schnelle , James Bottomley CC: linux-kernel@vger.kernel.org, linux-scsi , FUJITA Tomonori Subject: Re: [PATCH] [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference References: <867igc3w8r.fsf@deprecated.bitebene.org> In-Reply-To: <867igc3w8r.fsf@deprecated.bitebene.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4974 Lines: 115 On Sun, Mar 09 2008 at 14:41 +0200, Sven Schnelle wrote: > Hi, > > i'm facing the following kernel oops with the latest git: > > --------------------------------------8<-------------------------------------- > Stopping MD arrays...failed (no MD subsystem loaded). > Mounting root filesystem read-only...done. > Will now restart. > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > IP: [] __gdth_interrupt+0x96e/0xb7f > PGD 7e51e067 PUD 7e6a1067 PMD 0 > Oops: 0002 [1] PREEMPT SMP > CPU 3 > Modules linked in: nvidia(P) lm85 hwmon_vid hwmon [last unloaded: nvidia] > Pid: 0, comm: swapper Tainted: P 2.6.25-rc4-smp-00134-g84c6f60 #11 > RIP: 0010:[] [] __gdth_interrupt+0x96e/0xb7f > RSP: 0018:ffff81007fbefed8 EFLAGS: 00010086 > RAX: 0000000000000000 RBX: ffff81007e0dda68 RCX: 0000000000000002 > RDX: 00000000ffffffff RSI: 0000000000000000 RDI: ffffc20000330000 > RBP: ffff81007fbeff08 R08: 0000000000000000 R09: ffff81007f01de70 > R10: 0000000000000000 R11: 0000000000000050 R12: ffff810000b10480 > R13: ffff810000b104ff R14: ffff81007e214200 R15: 0000000000000009 > FS: 0000000000000000(0000) GS:ffff81007f802c80(0000) knlGS:0000000000000000 > CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b > CR2: 0000000000000000 CR3: 000000007e643000 CR4: 00000000000006e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > Process swapper (pid: 0, threadinfo ffff81007fbe6000, task ffff81007fbe4000) > Stack: 0000000000000046 ffff81007e8a93c0 0000000000000000 0000000000000000 > 0000000000000018 0000000000000003 ffff81007fbeff18 ffffffff8051ab9f > ffff81007fbeff48 ffffffff80258cc2 ffffffff8092e300 ffff81007e8a93c0 > Call Trace: > [] gdth_interrupt+0x10/0x12 > [] handle_IRQ_event+0x27/0x57 > [] handle_fasteoi_irq+0x9c/0xdc > [] do_IRQ+0x88/0xfc > [] ? default_idle+0x0/0x5f > [] ret_from_intr+0x0/0xa > [] ? default_idle+0x39/0x5f > [] ? default_idle+0x34/0x5f > [] ? default_idle+0x0/0x5f > [] ? cpu_idle+0xbf/0xf5 > [] ? start_secondary+0x3e0/0x3ef > > > Code: 00 04 eb 15 3c 17 75 11 41 0f b6 c5 48 c1 e0 05 41 80 a4 04 92 02 00 00 fb 41 c7 86 18 01 00 00 00 00 00 00 49 8b 86 c0 00 00 00 00 00 e9 92 01 00 00 66 89 43 24 41 8b 84 24 68 02 00 00 89 > RIP [] __gdth_interrupt+0x96e/0xb7f > RSP > CR2: 0000000000000000 > ---[ end trace e81e561a458e8791 ]--- > Kernel panic - not syncing: Aiee, killing interrupt handler! > Rebooting in 5 seconds.. > --------------------------------------8<-------------------------------------- > > From 2dc63f9f8e61fd1c89f8b4d9b2d174be1c3bfbe2 Mon Sep 17 00:00:00 2001 > From: root > Date: Sun, 9 Mar 2008 13:25:07 +0100 > Subject: > > This fixes a 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). > > So we have two choices here: > > a) Allocate the sense_buffer > b) surrounding all accesses to sense_buffer with some if (!internal_command) > > I'm using solution a, as this keeps code simpler. > > Signed-off-by: Sven Schnelle > --- > drivers/scsi/gdth.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c > index 27ebd33..0b2080d 100644 > --- a/drivers/scsi/gdth.c > +++ b/drivers/scsi/gdth.c > @@ -493,6 +493,12 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, > if (!scp) > return -ENOMEM; > > + scp->sense_buffer = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL); > + if (!scp->sense_buffer) { > + kfree(scp); > + return -ENOMEM; > + } > + > scp->device = sdev; > memset(&cmndinfo, 0, sizeof(cmndinfo)); > > @@ -513,6 +519,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, > rval = cmndinfo.status; > if (info) > *info = cmndinfo.info; > + kfree(scp->sense_buffer); > kfree(scp); > return rval; > } James and linux-scsi CCed. James it looks reasonable. It's a fallout from the sense_buffer separation patches. Reviewed-by: Boaz Harrosh I will submit solution b) above as part of my sense handling patchset. Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/