From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751320AbdBXUWx (ORCPT ); Fri, 24 Feb 2017 15:22:53 -0500 Received: from mail-io0-f180.google.com ([209.85.223.180]:33907 "EHLO mail-io0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751273AbdBXUWp (ORCPT ); Fri, 24 Feb 2017 15:22:45 -0500 Subject: Re: [GIT PULL] Block pull request for- 4.11-rc1 To: Linus Torvalds , Bart Van Assche References: <1D08B61A9CF0974AA09887BE32D889DA0A74F3@ULS-OP-MBXIP03.sdcorp.global.sandisk.com> <633d226d-cec3-a01f-a069-ffff307e9715@kernel.dk> <20170220073539.GA17687@lst.de> <1D08B61A9CF0974AA09887BE32D889DA0A8CBD@ULS-OP-MBXIP03.sdcorp.global.sandisk.com> <1487957968.2575.6.camel@sandisk.com> <0af4ed69-a9d0-44be-701c-ced76b84bfef@kernel.dk> Cc: "hch@lst.de" , "linux-kernel@vger.kernel.org" , "linux-block@vger.kernel.org" , "snitzer@redhat.com" From: Jens Axboe Message-ID: <0dc039e5-58c5-d328-0b81-bf9e7d782675@kernel.dk> Date: Fri, 24 Feb 2017 13:22:42 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <0af4ed69-a9d0-44be-701c-ced76b84bfef@kernel.dk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/24/2017 01:00 PM, Jens Axboe wrote: > On 02/24/2017 12:43 PM, Linus Torvalds wrote: >> On Fri, Feb 24, 2017 at 9:39 AM, Bart Van Assche >> wrote: >>> >>> So the crash is caused by an attempt to dereference address 0x6b6b6b6b6b6b6b6b >>> at offset 0x270. I think this means the crash is caused by a use-after-free. >> >> Yeah, that's POISON_FREE, and that might explain why you see crashes >> that others don't - you obviously have SLAB poisoning enabled. Jens >> may not have. >> >> %rdi is "struct mapped_device *md", which came from dm_softirq_done() doing >> >> struct dm_rq_target_io *tio = tio_from_request(rq); >> struct request *clone = tio->clone; >> int rw; >> >> if (!clone) { >> rq_end_stats(tio->md, rq); >> rw = rq_data_dir(rq); >> if (!rq->q->mq_ops) >> blk_end_request_all(rq, tio->error); >> else >> blk_mq_end_request(rq, tio->error); >> rq_completed(tio->md, rw, false); >> return; >> } >> >> so it's the 'tio' pointer that has been free'd. But it's worth noting >> that we did apparently successfully dereference "tio" earlier in that >> dm_softirq_done() *without* getting the poison value, so what I think >> might be going on is that the 'tio' thing gets free'd when the code >> does the blk_end_request_all()/blk_mq_end_request() call. >> >> Which makes sense - that ends the lifetime of the request, which in >> turn also ends the lifetime of the "tio_from_request()", no? >> >> So the fix may be as simple as just doing >> >> if (!clone) { >> struct mapped_device *md = tio->md; >> >> rq_end_stats(md, rq); >> ... >> rq_completed(md, rw, false); >> return; >> } >> >> because the 'mapped_device' pointer hopefully is still valid, it's >> just 'tio' that has been freed. >> >> Jens? Bart? Christoph? Somebody who knows this code should >> double-check my thinking above. I don't actually know the tio >> lifetimes, I'm just going by looking at how earlier accesses seemed to >> be fine (eg that "tio->clone" got us NULL, not a POISON_FREE pointer, >> for example). > > I think that is spot on. With the request changes for CDBs, for non > blk-mq, we know also carry the payload after the request. But since > blk-mq never frees the request, the above use-after-free with poison > will only happen for !mq. Caching 'md' and avoiding a dereference of > 'tio' after calling blk_end_request_all() will likely fix it. > > Bart, can you test that? Bart, I pushed a fix here: http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=61febef40bfe8ab68259d8545257686e8a0d91d1 -- Jens Axboe