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 09336C677F1 for ; Thu, 12 Jan 2023 22:01:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5499F8E0002; Thu, 12 Jan 2023 17:01:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F9B68E0001; Thu, 12 Jan 2023 17:01:40 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3C0AF8E0002; Thu, 12 Jan 2023 17:01:40 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 2A86E8E0001 for ; Thu, 12 Jan 2023 17:01:40 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id D0807C0E5F for ; Thu, 12 Jan 2023 22:01:39 +0000 (UTC) X-FDA: 80347519518.29.07FB721 Received: from mail-yw1-f174.google.com (mail-yw1-f174.google.com [209.85.128.174]) by imf23.hostedemail.com (Postfix) with ESMTP id AA9B4140023 for ; Thu, 12 Jan 2023 22:01:36 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=A+wS4Fao; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.128.174 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673560896; a=rsa-sha256; cv=none; b=f+8S5L8XeMV99ZGtCqCCFrzxLwDxBtVP0y0ozdN/HppG/ES14OVdJToEcZ6CqrI8kbjJ+a BTtvshnJer/G4rV9Y/rrJOV/Q3VwO/JF3LuJHRSFhmloeNOhmKDNnZYRawLbZK6qllA3Uu 3OkZ+qzL2jnqeUp8jhib6b+7ywLizVk= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=A+wS4Fao; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.128.174 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673560896; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=w5TcqIxkG85wy5A28OaJ2ye2/9ChOspqn1Q7cVOdJn0=; b=tnAUBYeQBYKhDAUBdcKJbTReSUKysBsOQn4OQbMj0AfO/mMd62tqqczeKH/LywtBOIquYW 9lvSxVoZWGrChlS4PyZ+D8/sf3Mzo+kHMdIr65DDybB2P4A6UYbeDFEyBgVBvpElWiStOS oQzjHKdq50f1XYK6UB68JHCnFOC0u/A= Received: by mail-yw1-f174.google.com with SMTP id 00721157ae682-4a263c4ddbaso260244217b3.0 for ; Thu, 12 Jan 2023 14:01:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=w5TcqIxkG85wy5A28OaJ2ye2/9ChOspqn1Q7cVOdJn0=; b=A+wS4FaoYpgPCd0w38jm84CFe8f75fRTrIXDBdk9m3jD7iCzlcRn9Rg9OcYCILRwjY iB4NDneF/jHazFp/Bxz/neGxg4+pdPgkWbCLsQHnRyFZXHgJAd50tpB2dbtp+qklrOYt A/sx3rwgs8x8SNpxqKkW201PO03W/uVUg2V2Y/4yZbEaM6jbhih8pzMWlL6Tb8E2jc6H 3BbldDBa/6K/M0VTcHxxzFdEGQBmLRDDfn9DnLX45gCkN7sTF+vuNRkD5zV9Iv4oR1OX eezNaDIptisgCNpOEMMof8E6OI9peoCjEVOEc9Fa7ILgUdurmLGE8RwzxH4X3jAKAJWH CEKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=w5TcqIxkG85wy5A28OaJ2ye2/9ChOspqn1Q7cVOdJn0=; b=VbD/3Y0QRU+FzN3slZehDUCG1qAWY0nr5JZHYh40GkyJ3GQINb14FTkFswqHUOsEju jVkLNYa+zC44JpSKdp5AaF6wjfJqLqe2QOmZCCv3x58/jOOVcD+hvM/ImAW/KJtYXg9+ aD2QjyWlq61dstclzlpNEbVmA7dFsqVUjMeWmgqGf017Yk5OIrG2Qd73jLwoScYjY/Kq 7LXsoXVMCEK3m9Tgry3YIiiqIZXJeyYkvBY9tLn5g/XCkopoXd9JNJxebXKx5Ror7YC6 IdxqSqvNp+i8+PVrPg+Ku/+NIk+rf5qI4QuRxzNDLJPWQ5fWNBCS/Qg6Rbcl1GSWIK73 nAAw== X-Gm-Message-State: AFqh2kp+dJfWcPnTGspAPD8ruMxHCuvh5UsXwZKTBMT+UkduB0hzdjvY zXDmVvbnHpzrff3cDXsu0o/NKYmuFMf6gcX1J++7yw== X-Google-Smtp-Source: AMrXdXtVXRFLY7wCC4gUeokPTi/KXPVvtemxjT2oHq9FYY9pfpQH+oC59h5/OYQ+da0pOshzN9hZOZho5femd/TbPxg= X-Received: by 2002:a81:1352:0:b0:4dc:4113:f224 with SMTP id 79-20020a811352000000b004dc4113f224mr268784ywt.455.1673560895606; Thu, 12 Jan 2023 14:01:35 -0800 (PST) MIME-Version: 1.0 References: <20230107080702.4907-1-hdanton@sina.com> <20230108222548.698721-1-kamatam@amazon.com> <20230108234917.5322-1-hdanton@sina.com> In-Reply-To: From: Suren Baghdasaryan Date: Thu, 12 Jan 2023 14:01:24 -0800 Message-ID: Subject: Re: another use-after-free in ep_remove_wait_queue() To: Hillf Danton Cc: Munehisa Kamata , hannes@cmpxchg.org, ebiggers@kernel.org, mengcc@amazon.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Queue-Id: AA9B4140023 X-Rspamd-Server: rspam01 X-Stat-Signature: 536azuqmxtzgia3wp6gksnttoyb5sja9 X-HE-Tag: 1673560896-270535 X-HE-Meta: U2FsdGVkX18lH5o07/tQFOPTB6Dn6kAy8TEYP8gsffftc1EGruVT7F4q+dQf64bg0TWgcnqwaYQGsl+Di9Mc+uRtOQdU7Xb6QVlMnJE6HV0eJo7Hkb/ek93s+a0b4yFw3vitqc07gxcWFQkYcP8bRZORoOf1MlU2D9yVAwupayH/JG8bYAT/rBSQmPQtLN24K7XhaBlnYekYdWTBsa0apRGSmwA4g2M4/Isu/uXEZkUoBodDu7o2X/bcNR2KGxP5twNMr4nQVv3nZ4/ALN34GAB/anES5MWRyC4eJXzrQDTUn+eDqgiefkbNkKZdYiYrRwSwvdrjy2upTZ5xurpfPiU/JElxDJqgns7XDmnaHmxg6Wu+7+z6QIvFp8cCjap8WYR0PgQGexUsyX1YBUOek3VswmbWZVoMlJ3T0w12KB0tvlEpqYxqcM4XtnYQKXln79YRA/1AIXc298rB/QRDy3JO/HESCIZgtVZkuOYpw0x4jS2idsxDiThXhmoSaTmi5pBkZr++szP3sCdNfpirpe0bfmP6GAfLzl+02SME2Wl4dzV97XYyJ6eG42HqGQbAJRYHPaAWzgfTBS9U3UXED/7z/jh9XQriFellE8CRItHKZ4Z98k54VmB187jtfICtYyslFjRD6eDvO6n7IExIAUSKAH/lec4A69/emsim1poY2zzmsH4TqvyG3wTwB+neNdSX3JMMtE/zo9Jkmhoh29JYobI9V5hYMYQDok4mdXKmO651gk4WYlKDzgTAhT6heC3tQNFES0fvg66/C9iwauPGjfclZXbZy0a0h0ihjlt+yEsyzIWcHO02y1vXIUPWKPVggy9CVZqwOAGWC092Bgrv65dN995aaG/tSn9JYSE20B/vRQVRaNiknH1jwbr1K3Haa+FoEYo4/ZsDgyK4q8Ff4HiOs1PiXQLaEKLXMW9jonPLCA27Fw5aoJ42F1kBV8sxnbxbd1NolvSV8p3 XNnSDjlN F+y9v/Hxkq6IQXtfavkrRBIywkCmlqNuT487F9VzlsiYilF6qv0YU4OmPIA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000336, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jan 9, 2023 at 7:06 PM Suren Baghdasaryan wrote: > > On Mon, Jan 9, 2023 at 5:33 PM Suren Baghdasaryan wrote: > > > > On Sun, Jan 8, 2023 at 3:49 PM Hillf Danton wrote: > > > > > > On 8 Jan 2023 14:25:48 -0800 PM Munehisa Kamata wrote: > > > > > > > > That patch survived the repro in my original post, however, the waker > > > > (rmdir) was getting stuck until a file descriptor of the epoll instance or > > > > the pressure file got closed. So, if the following modified repro runs > > > > with the patch, the waker never returns (unless the sleeper gets killed) > > > > while holding cgroup_mutex. This doesn't seem to be what you expected to > > > > see with the patch, does it? Even wake_up_all() does not appear to empty > > > > the queue, but wake_up_pollfree() does. > > > > > > Thanks for your testing. And the debugging completes. > > > > > > Mind sending a patch with wake_up_pollfree() folded? > > > > I finally had some time to look into this issue. I don't think > > delaying destruction in psi_trigger_destroy() because there are still > > users of the trigger as Hillf suggested is a good way to go. Before > > [1] correct trigger destruction was handled using a > > psi_trigger.refcount. For some reason I thought it's not needed > > anymore when we placed one-trigger-per-file restriction in that patch, > > so I removed it. Obviously that was a wrong move, so I think the > > cleanest way would be to bring back the refcounting. That way the last > > user of the trigger (either psi_trigger_poll() or psi_fop_release()) > > will free the trigger. > > I'll check once more to make sure I did not miss anything and if there > > are no objections, will post a fix. > > Uh, I recalled now why refcounting was not helpful here. I'm making > the same mistake of thinking that poll_wait() blocks until the call to > wake_up() which is not the case. Let me think if there is anything > better than wake_up_pollfree() for this case. Hi Munehisa, Sorry for the delay. I was trying to reproduce the issue but even after adding a delay before ep_remove_wait_queue() it did not happen. One thing about wake_up_pollfree() solution that does not seem right to me is this comment at https://elixir.bootlin.com/linux/latest/source/include/linux/wait.h#L253: `In the very rare cases where a ->poll() implementation uses a waitqueue whose lifetime is tied to a task rather than to the 'struct file' being polled, this function must be called before the waitqueue is freed...` In our case we free the waitqueue from cgroup_pressure_release(), which is the handler for `release` operation on cgroup psi files. The other place calling psi_trigger_destroy() is psi_fop_release(), which is also tied to the lifetime to the psi files. Therefore the lifetime of the trigger's waitqueue is tied to the lifetime of the files and IIUC, we should not be required to use wake_up_pollfree(). Could you please post your .config file? I might be missing some configuration which prevents the issue from happening on my side. Thanks, Suren. > > > > > > [1] https://lore.kernel.org/lkml/20220111232309.1786347-1-surenb@google.com/ > > > > Thanks, > > Suren. > > > > > > > > Hillf