From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757960Ab1JRPps (ORCPT ); Tue, 18 Oct 2011 11:45:48 -0400 Received: from mtagate2.uk.ibm.com ([194.196.100.162]:59068 "EHLO mtagate2.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755519Ab1JRPpq (ORCPT ); Tue, 18 Oct 2011 11:45:46 -0400 Date: Tue, 18 Oct 2011 17:45:42 +0200 From: Heiko Carstens To: "Jun'ichi Nomura" Cc: James Bottomley , Steffen Maier , "linux-scsi@vger.kernel.org" , Jens Axboe , Hannes Reinecke , Linux Kernel , Alan Stern , Thadeu Lima de Souza Cascardo , "Taraka R. Bodireddy" , "Seshagiri N. Ippili" , "Manvanthara B. Puttashankar" Subject: Re: [GIT PULL] Queue free fix (was Re: [PATCH] block: Free queue resources at blk_release_queue()) Message-ID: <20111018154542.GB3869@osiris.boeblingen.de.ibm.com> References: <1317183038.17578.9.camel@dabdike.hansenpartnership.com> <4E832A55.30709@kernel.dk> <1317219074.19034.4.camel@dabdike.hansenpartnership.com> <4E832BD2.50409@kernel.dk> <4E96E2FF.6070601@linux.vnet.ibm.com> <1318608184.3018.42.camel@dabdike.int.hansenpartnership.com> <4E9BEB68.1050707@ce.jp.nec.com> <1318860403.4794.12.camel@dabdike.int.hansenpartnership.com> <4E9D7FA8.9000000@ce.jp.nec.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E9D7FA8.9000000@ce.jp.nec.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 18, 2011 at 10:31:20PM +0900, Jun'ichi Nomura wrote: > On 10/17/11 23:06, James Bottomley wrote: > > On Mon, 2011-10-17 at 17:46 +0900, Jun'ichi Nomura wrote: > >> On 10/15/11 01:03, James Bottomley wrote: > >>> On Thu, 2011-10-13 at 15:09 +0200, Steffen Maier wrote: > >>>> Initially, we encountered use-after-free bugs in > >>>> scsi_print_command / scsi_dispatch_cmd > >>>> http://marc.info/?l=linux-scsi&m=130824013229933&w=2 > >> > >> It is interesting that both this and the older report > >> got oopsed in scsi_log_send(), while there are other > >> dereferences of 'cmd' around scsi_dispatch_cmd(). > >> Are there any reason they are special? Just by accident? > > > > Right, that's why it looks like the command area got freed rather than > > the command pointer was bogus (6b is a poison free pattern). Perhaps if > > the reporter could pin down the failing source line, we'd know better > > what was going on? > > Yeah, that might be useful. The struct scsi_cmnd that was passed to scsi_log_send() was already freed (contents completely 6b6b6b...). Since SLUB debugging was turned on we can see that it was freed from __scsi_put_command(). Not too much of a surprise. > One remote possibility I imagined is if the submitting process > took very long after blk_start_request until scsi_dispatch_cmd > and timeout handler kicks in, can cmd be freed? > > Also Tejun's report here could be related to possible data corruption: > [PATCH] block: make gendisk hold a reference to its queue > https://lkml.org/lkml/2011/10/16/148 > > Though I haven't hit the reported oops myself, > a process closing a removed device may modify freed memory. > And his patch will fix it. > > So if the problem is easily reproducible, I think it's worth > trying his patch to see if the problem disappear. In addition it might be worthwhile to apply this small slub patch as well, so that we get a full trace of the caller in the object which was freed: --- mm/slub.c | 11 +++++++++++ 1 file changed, 11 insertions(+) --- a/mm/slub.c +++ b/mm/slub.c @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -191,11 +192,14 @@ static LIST_HEAD(slab_caches); /* * Tracking user of a slab. */ +#define SAVE_TRACE_NUM 30 struct track { unsigned long addr; /* Called from address */ int cpu; /* Was running on cpu */ int pid; /* Pid context */ unsigned long when; /* When did the operation occur */ + unsigned long trace[SAVE_TRACE_NUM]; + unsigned long long stamp; }; enum track_item { TRACK_ALLOC, TRACK_FREE }; @@ -397,10 +401,17 @@ static void set_track(struct kmem_cache struct track *p = get_track(s, object, alloc); if (addr) { + struct stack_trace trace = { + .max_entries = SAVE_TRACE_NUM, + .entries = p->trace, + }; + p->addr = addr; p->cpu = smp_processor_id(); p->pid = current->pid; p->when = jiffies; + p->stamp = get_clock(); + save_stack_trace(&trace); } else memset(p, 0, sizeof(struct track)); }