From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6072C433DF for ; Mon, 22 Jun 2020 12:37:05 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A4A8C2071A for ; Mon, 22 Jun 2020 12:37:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A4A8C2071A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1438E6B0002; Mon, 22 Jun 2020 08:37:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F6336B0003; Mon, 22 Jun 2020 08:37:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 00BBD6B0005; Mon, 22 Jun 2020 08:37:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0104.hostedemail.com [216.40.44.104]) by kanga.kvack.org (Postfix) with ESMTP id DCDF56B0002 for ; Mon, 22 Jun 2020 08:37:04 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 39D4A180AD807 for ; Mon, 22 Jun 2020 12:37:04 +0000 (UTC) X-FDA: 76956797568.29.town37_48089e726e32 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin29.hostedemail.com (Postfix) with ESMTP id 1297018086E22 for ; Mon, 22 Jun 2020 12:37:04 +0000 (UTC) X-HE-Tag: town37_48089e726e32 X-Filterd-Recvd-Size: 6020 Received: from mail-pj1-f67.google.com (mail-pj1-f67.google.com [209.85.216.67]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Mon, 22 Jun 2020 12:37:03 +0000 (UTC) Received: by mail-pj1-f67.google.com with SMTP id h95so8550818pje.4 for ; Mon, 22 Jun 2020 05:37:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=XBnvpozQh8NlbXbb2Id9vk4JGCe+27nJySmmknsgIeQ=; b=aGHF6oDTEH8bV5tuLvHgNnM5JuGWRGLxy4k2gqjOp5m70hgZW1MIo7gtKsMSSHSqd8 KKLfDm28JJfsE7XrqfxJwnDbi1Uw4hszTs6kdJ+vr8F8EaMbvseaKTtXXnh0Cs7UFkpw feFlRUqkqxHAzqxuV0uav+NHiN9CjAbA7bgp7UpPQIzHiCXM4wMFOftsb7Y8g14mv6Bi cabWqfx8QGIfjq4NlvXAG8hRmgArK1euiWNqDgOEUl/WQIKt6tm1mQHp49RpfZsL+Wig otR2WMFQ3TxyysWAtLNSokHfQVgaUb+6h29+v3Gsp8SFI3oIGeBhhIVACC52cniG5x4r t6dQ== X-Gm-Message-State: AOAM530iQlBUmkfYzDR4nrOE66iBkr+dZlE7mo8NdqoAotrO16iKDchA BPXr8g+jaTSVMeWDkdzeV4g= X-Google-Smtp-Source: ABdhPJyrKfaLQ3639cZrtv1SHn9cay5HXy5MKWY132MuQMsHcqSjbmonrxxCryowUAKdi5d5SomNKg== X-Received: by 2002:a17:90a:b30d:: with SMTP id d13mr17854178pjr.181.1592829422425; Mon, 22 Jun 2020 05:37:02 -0700 (PDT) Received: from 42.do-not-panic.com (42.do-not-panic.com. [157.230.128.187]) by smtp.gmail.com with ESMTPSA id y4sm13839956pfr.182.2020.06.22.05.37.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Jun 2020 05:37:00 -0700 (PDT) Received: by 42.do-not-panic.com (Postfix, from userid 1000) id 9C42940430; Mon, 22 Jun 2020 12:36:59 +0000 (UTC) Date: Mon, 22 Jun 2020 12:36:59 +0000 From: Luis Chamberlain To: Bart Van Assche Cc: axboe@kernel.dk, viro@zeniv.linux.org.uk, gregkh@linuxfoundation.org, rostedt@goodmis.org, mingo@redhat.com, jack@suse.cz, ming.lei@redhat.com, nstange@suse.de, akpm@linux-foundation.org, mhocko@suse.com, yukuai3@huawei.com, martin.petersen@oracle.com, jejb@linux.ibm.com, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Omar Sandoval , Hannes Reinecke , Michal Hocko , syzbot+603294af2d01acfdd6da@syzkaller.appspotmail.com Subject: Re: [PATCH v7 6/8] blktrace: fix debugfs use after free Message-ID: <20200622123659.GV11244@42.do-not-panic.com> References: <20200619204730.26124-1-mcgrof@kernel.org> <20200619204730.26124-7-mcgrof@kernel.org> <75c3a94d-dcd1-05e4-47c6-db65f074136a@acm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <75c3a94d-dcd1-05e4-47c6-db65f074136a@acm.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspamd-Queue-Id: 1297018086E22 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam02 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Jun 20, 2020 at 10:31:51AM -0700, Bart Van Assche wrote: > On 2020-06-19 13:47, Luis Chamberlain wrote: > > This goes tested with: > ^^^^ > got? > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > index 7ff2ea5cd05e..e6e2d25fdbd6 100644 > > --- a/kernel/trace/blktrace.c > > +++ b/kernel/trace/blktrace.c > > @@ -524,10 +524,18 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, > > if (!bt->msg_data) > > goto err; > > > > - ret = -ENOENT; > > - > > - dir = debugfs_lookup(buts->name, blk_debugfs_root); > > - if (!dir) > > +#ifdef CONFIG_BLK_DEBUG_FS > > + /* > > + * When tracing whole make_request drivers (multiqueue) block devices, > > + * reuse the existing debugfs directory created by the block layer on > > + * init. For request-based block devices, all partitions block devices, > ^^^^^^^^^^^^^^^^^^^^^ > It seems like a word is missing from the comment? Or did you perhaps > want to refer to "all partition block devices"? Yes, the later. > > + * and scsi-generic block devices we create a temporary new debugfs > > + * directory that will be removed once the trace ends. > > + */ > > + if (queue_is_mq(q) && bdev && bdev == bdev->bd_contains) > > + dir = q->debugfs_dir; > > + else > > +#endif > > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > Can it happen that two different threads each try to set up block > tracing and hence that debugfs_create_dir() fails because a directory > with name buts->name already exists? Great question, the answer is no. The reason is that we first use the mutex and then we check for q->blk_trace. If you hold the lock *and* you have checked for q->blk_trace and its NULL, you are sure you should not have a duplicate. Its why the commit log mentioned: The new clarifications on relying on the q->blk_trace_mutex *and* also checking for q->blk_trace *prior* to processing a blktrace ensures the debugfs directories are only created if no possible directory name clashes are possible. These clarifications were prompted through discussions with Jan Kara on the patches he posted which you CC'd me on. I agreed with his patch *but* I suggested it would hold true only if check for the q->blk_trace first, and this is why my patch titled "blktrace: break out of blktrace setup on concurrent calls" got merged prior to Jan Kara's "blktrace: Avoid sparse warnings when assigning q->blk_trace". > > bt->dev = dev; > > @@ -565,8 +573,6 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, > > > > ret = 0; > > err: > > - if (dir && !bt->dir) > > - dput(dir); > > if (ret) > > blk_trace_free(bt); > > return ret; > > Shouldn't bt->dir be removed in this error path for make_request drivers? If there is an error, bt->dir will be removed still, as I never modified the removal of bt->dir in this patch. Luis