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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 114D1C433FE for ; Wed, 30 Nov 2022 15:23:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 985178E0001; Wed, 30 Nov 2022 10:23:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 934B36B0078; Wed, 30 Nov 2022 10:23:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7FD528E0001; Wed, 30 Nov 2022 10:23:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 72E6A6B0075 for ; Wed, 30 Nov 2022 10:23:48 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 14CEE12111A for ; Wed, 30 Nov 2022 15:23:48 +0000 (UTC) X-FDA: 80190478536.21.AA389E5 Received: from mail-il1-f181.google.com (mail-il1-f181.google.com [209.85.166.181]) by imf30.hostedemail.com (Postfix) with ESMTP id 651C380011 for ; Wed, 30 Nov 2022 15:23:47 +0000 (UTC) Received: by mail-il1-f181.google.com with SMTP id m15so8248774ilq.2 for ; Wed, 30 Nov 2022 07:23:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=MVSvVeCDGQ1o47gYLtN8R+awQ9lS7VAhyH71o2xLC54=; b=ctCk0Dd6GzAls1JzBIxBk3DiSB9qYtO/HUeatCrvfQP+Kd/x38VyYHbGjvMu21RNH2 XPVqP5WnCYBDrtJRf6yQdJbQHyHzCF4Ilyg0ou4AXQ5x0ECKWT6hs7QGdFhwGfbL8S89 VaN2X8kSnHxYl7xt0FZDyKaa6axIjIyEf3yudcU3GmWc1Qi/bO/hxPxUSPvwEYL25zgN fYSc2O894eFB+ql7gLFD9tLcPNV1OFNOAqFeiQdH7ZvkijH5E64ZuBq9KV7B7zNIsASP 6lyHiU4g+neMI8hcv1LCpJeDlMuLgkhZZCTfWAsosdJTAC5CVztGGjtJtILnLXvg5uCj MKXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MVSvVeCDGQ1o47gYLtN8R+awQ9lS7VAhyH71o2xLC54=; b=NTAItLrkuCqKwMbc4XjfScWHNx4z2grb1uaNWbcQrnqyD3/kn4fPZ2GEnoyytsKDTN hMKgb/VA4T0Lysgh0yAnj8Y6bO/XbarYkNpYxkKBYFsSPGTUuwn7DjKOdJxaRWY9o7QL hkRz3Du8Jrg/7AoLzvcAbdBp0FM+mYYBaUEsHA/9jCAKPGdjR1CvSR23wO0aDwOOToQB GUkJ8LDvC05nKXT1AmME0bixeU8YJyLdKZ1/z7/YZlG1LjSpgk1mNzNhrivzFBJ4Xt9F aPjySgMcawf4Uoupeo5nXkvyaamrE4uk/hON+MlS5Gi6ctfFrs7R8ZiRU+btlRILQ2Te s9HQ== X-Gm-Message-State: ANoB5pkABeqaYa269y7fkA1HJx4Rlb7Lpn1qhmJ1Moq7pBvowTjG0zQq 37IeU4YEY71WTvfGTkY6RPcLuw== X-Google-Smtp-Source: AA0mqf4F4EP7I7z/iy26h49+AyTmYS1hRiveg8HEWlXm02uDHZjzR6GxDz6Hbi2m/dy6XeXKC+V4Ow== X-Received: by 2002:a05:6e02:d94:b0:302:a008:83e5 with SMTP id i20-20020a056e020d9400b00302a00883e5mr18559072ilj.293.1669821826216; Wed, 30 Nov 2022 07:23:46 -0800 (PST) Received: from [192.168.1.94] ([207.135.234.126]) by smtp.gmail.com with ESMTPSA id a10-20020a5ec30a000000b006bbddd49984sm631990iok.9.2022.11.30.07.23.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Nov 2022 07:23:45 -0800 (PST) Message-ID: <34932e06-7336-3f17-9bc4-0fc67130d561@kernel.dk> Date: Wed, 30 Nov 2022 08:23:44 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH-block v2] bdi, blk-cgroup: Fix potential UAF of blkcg Content-Language: en-US To: =?UTF-8?Q?Michal_Koutn=c3=bd?= , Waiman Long Cc: Tejun Heo , cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Ming Lei , Andy Shevchenko , Andrew Morton , Hillf Danton , Chaitanya Kulkarni , Bart Van Assche , Josef Bacik , Yi Zhang References: <20221129203400.1456100-1-longman@redhat.com> <20221130151639.GE27838@blackbody.suse.cz> From: Jens Axboe In-Reply-To: <20221130151639.GE27838@blackbody.suse.cz> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669821827; a=rsa-sha256; cv=none; b=dSFC0PciO0MIv20hGWKAuAYPYMr6qQy6cOppuAC4i4SfJ1Q2WSELrFxGnZKD8u0YKdAuKG yysZsO7aZGTbeA2Wgl/g5TfK3FdPieaOdUbzF//7Tg12vCsvdM5DRka3CK1i1gyMdtQd5N x9U+bmG1WqCWKRCOBncqwqTYLfAgJUU= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel-dk.20210112.gappssmtp.com header.s=20210112 header.b=ctCk0Dd6; dmarc=none; spf=pass (imf30.hostedemail.com: domain of axboe@kernel.dk designates 209.85.166.181 as permitted sender) smtp.mailfrom=axboe@kernel.dk ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669821827; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MVSvVeCDGQ1o47gYLtN8R+awQ9lS7VAhyH71o2xLC54=; b=YUlQeMG3stUIh+MZ4dkVYv+3Cr2dd8rSxwpgoqIRiVoD1vs43+Tzr2+AaWU9LKeUU+Q2ym R7havQrplefLr9lf75B9OoFg4PH3XZudVFOfTOijkPslRdGBO/8IjJGZOhOVE+Vt5zK8jf WrNR3cFxswl2cxtNsT23Zoe+vbjTncM= X-Rspamd-Queue-Id: 651C380011 X-Rspam-User: Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel-dk.20210112.gappssmtp.com header.s=20210112 header.b=ctCk0Dd6; dmarc=none; spf=pass (imf30.hostedemail.com: domain of axboe@kernel.dk designates 209.85.166.181 as permitted sender) smtp.mailfrom=axboe@kernel.dk X-Rspamd-Server: rspam09 X-Stat-Signature: nmdkhhrq65srzoxqwheo4ih57434ki3o X-HE-Tag: 1669821827-101008 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 11/30/22 8:16?AM, Michal Koutn? wrote: > On Tue, Nov 29, 2022 at 03:34:00PM -0500, Waiman Long wrote: >> The reproducing system can no longer produce a warning with this patch. >> All the runnable block/0* tests including block/027 were run successfully >> without failure. > > Thanks for the test! > >> @@ -1088,7 +1088,15 @@ static void blkcg_destroy_blkgs(struct blkcg *blkcg) >> >> might_sleep(); >> >> - css_get(&blkcg->css); >> + /* >> + * blkcg_destroy_blkgs() shouldn't be called with all the blkcg >> + * references gone and rcu_read_lock not held. >> + */ >> + if (!css_tryget(&blkcg->css)) { >> + WARN_ON_ONCE(!rcu_read_lock_held()); >> + return; >> + } > > As I followed the previous discussion, the principle is that obtaining a > reference or being inside an RCU read section is sufficient. > > Consequently, I'd expect the two situations handled equally but here the > no-ref but RCU bails out. (Which is OK because blkg_list must be empty?) > > However, the might_sleep() in (non-sleepable) RCU reader section combo > makes me wary anyway (not with the early return but tools would likely > complain). > > All in all, can't the contract of blkcg_destroy_blkgs() declare that > a caller must pass blkcg with a valid reference? (The body of > blkcg_destroy_blkgs then wouldn't need to get neither put the inner > reference). Totally agree, the proposed patch feels more like a hacky workaround rather than a true solution. Either the contract should be that it's ALWAYS entered with RCU lock held and hence the tryget is fine, OR that a reference always is held when entered. I'm going to revert the offending patch for now, and then we can queue up a proper patch when that exists. -- Jens Axboe