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 13D11C636CC for ; Mon, 13 Feb 2023 23:50:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 509C86B0072; Mon, 13 Feb 2023 18:50:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B9866B0075; Mon, 13 Feb 2023 18:50:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3816B6B0078; Mon, 13 Feb 2023 18:50:19 -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 26F256B0072 for ; Mon, 13 Feb 2023 18:50:19 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id EA327C0894 for ; Mon, 13 Feb 2023 23:50:18 +0000 (UTC) X-FDA: 80463914916.03.F1F0069 Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf29.hostedemail.com (Postfix) with ESMTP id 24BF512000F for ; Mon, 13 Feb 2023 23:50:16 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=YX6UBeGB; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 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=1676332217; 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=/FT+wytgM0VWNp2khoBQRK6+3iQndNyjsjvLj9/BF0o=; b=Nw/p+wrCEbQ6j47C54hKOthH4CgddKuWCKrb0WDVU6Ms5kb3hPE4MLqUQhyNnNP+CVqnlr N/S30/1ShsUusIG10dYZlBGCEmM7h6b3g+xtC4Cs+lMyPYrRZ8wH5A/M/cuGZJEbv2nXwK eqOiXK0CZjL652eDxpzDtoL6/N45ATs= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=YX6UBeGB; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 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=1676332217; a=rsa-sha256; cv=none; b=MkcYzdjOta4X3yClDy4LmUSfkMkhPUlu7KryL5cIw9YIKULZUKUZg6Bd4ORr3wpxUIT1Bs /3ljJXe26JoOMMTAjfYKclmxtXGMOeHtlVw/ropPiN5APjFwTNr8g/xL0N1J04rIHUGYxb vL5C6sLKFKg/cQ09NAN/2IwDnfupzO8= Received: by mail-yb1-f169.google.com with SMTP id s203so11232612ybc.11 for ; Mon, 13 Feb 2023 15:50:16 -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=/FT+wytgM0VWNp2khoBQRK6+3iQndNyjsjvLj9/BF0o=; b=YX6UBeGBrDsSaPExb1/m/al7La5LF8ds9CzmACNP5wZ3A7aRy0S5n5EYAboZE1AVYk LKnS6iCpkm+NCAJRfgXMBPAAee82MANl6z9Egn3HBQL8+DDTloIoKF1hf0rqhrRR3Z4Q HDEumfMrljpTLskBJLusW4Aux5ztIp5pSnmGrygyRElhRq2bGknFQFca17kbpN+mn/uI /pNjKVTVOxwi4EedxjiXrKLI/5GAUFJ1uKqMaYfPHvKEbtrVEK7s574QvJ6sxN6KuPpK GF3ID5mQGWn/fj5+twl9PJdyzo8T2o7cl8RC5jgz4gthQhBLpoFPgZzJzgFk0EPvzo7B yg6Q== 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=/FT+wytgM0VWNp2khoBQRK6+3iQndNyjsjvLj9/BF0o=; b=H1l0d3FstNaixH5ObLzKL76O6jpZDKqPBxcgFhdXpabINefhtz7L58G/uAuTDmmV5m NBXESE8fmyf1+ieir7YOQIcO6LQ1yZpQySGhx5BOZoqSW5thkouBkPI5DniVR1mEG4W+ KwiZPNlkjZ7kCpwBkVKkiGWZylyDsKr5BSsVjXWpFqRAzaMlQ70cIdPjj7jhMdqC2GTH P/fGg8vMOz4vP1NsUEeXNsFnfN0h88+RYRgxJs/NVkFJOxSfJhNs5IRcENo/i3mmgnk0 W8Q6WtensiwhoPpxL29UcxSoq+LPsKBJkOz7ddWEjUaD8oEvkzPWvCk085SHCLcTXmdc SxwQ== X-Gm-Message-State: AO0yUKVFs69RueirGxfzNhylVIsWjAhQXohdHQnMmY0PHNNphZpOOKDG TODwYeMloUn07Wo+pelfb3QyFd/z465BxIcq75KyOA== X-Google-Smtp-Source: AK7set9YvqKjU6/vp0rfYhefce64PiRsLuVzPrjuRi58c5zUNgAb80Bye1pFfse0I4QrxilSLOi+TmkNQydBgyh36MQ= X-Received: by 2002:a05:6902:52a:b0:900:c3fd:a078 with SMTP id y10-20020a056902052a00b00900c3fda078mr76541ybs.657.1676332215989; Mon, 13 Feb 2023 15:50:15 -0800 (PST) MIME-Version: 1.0 References: <20230202030023.1847084-1-kamatam@amazon.com> In-Reply-To: From: Suren Baghdasaryan Date: Mon, 13 Feb 2023 15:50:05 -0800 Message-ID: Subject: Re: [PATCH] sched/psi: fix use-after-free in ep_remove_wait_queue() To: Eric Biggers Cc: Munehisa Kamata , hannes@cmpxchg.org, hdanton@sina.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, mengcc@amazon.com, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 24BF512000F X-Stat-Signature: 47zz93zc5bixsczuj68ojg3s1a6z3r6k X-HE-Tag: 1676332216-258472 X-HE-Meta: U2FsdGVkX18lw4yQk8pOSSGtX9nuI8Kbm2eibig5Rrh+/5RjqpyxHcexOgwvVtG+TPZstZPBgiDGGfN4ktoJDWA+QccOIWmA8iizswomWIewVlnexPL6m7FbaD/Mvqxnh9EPoGgvm9xpiXX05WfwGhG4vqNso2fWBI1w67vXoT+v7JHJsPYLODGl5YXI2rGtyskn6FpLLaHzqz+xIAOO6sM5MiY1j+iu1m6eYMyqAdFJfIFTddbORtK/MQSpx+jSfea3VFQ3J+WViyBfcPIjO6zMztTqJMxoN+v1RUdCAnx6eRC/X7I8cf9TQ+37N++emRIwi3s4Hli7C37R9pOgwnEjA7gmG0ZvZx0HiB87wg8sVEAH+rontlgJ0zvQQa5Ve0amiI/SrxrrkR0WzOaNO6Yi8YM2dn+qfHBUyObtjbfLc6GsEUoHxhaoXcKOKdJ6UATyVTRmvMpLJkjfGC+8r0HvAui7KIgqNHSz317TCWsrV4ei67xwERx0qxyurzw4QpgNBJAZpmFZe7RBVNbd2b2+XvFac/2+M67B9qQzWuk+hQfHzLe1AjsbcZj0300TOZRwDQls3T6ejwE4seeoytLoTuMhyppkSzjA/SeRcdolvy0eWXAG5xRk/TTmWzkIuy+r+3XwZmQqoVcTQmLflILE1utwRAxTGB0NNsNKotiY+YMPxPQf6taIkxodjKd9ixnpu54JHxAYm7S9GMFEX9RGl+rbxHKqLT5uabRJMFliw9SZfI/Q6iRHH2vYpGszKEZS05se3LAvuHeuMAcE59584Qpdj0r+MU1sIBsM+h8c5VRy9zcv9a109BbaLXAtPlPGVzcQbCs/jMUacLt2J2MKJDb4uTALqeHNHJDGoGqMlMe5Z9uskqvIHX93EibJvw/ksxrdTG4K0h9sL8ibFiUB+8OjGbmtUPslXeFTXAqbKBLN5MdDjEsRsjjuVHYjfy+BL4rEhEXNLuk0jeb F+hcxE6J APpbgpjJocfwGtSn++ncAJi7fDmx3C++aBMZgeGf0oHEoTEI20UzqbaGirc+JfBpfYkWUl4U15bx5L/e/u5zP1b2gFpuiQ6zn3Bfc+pnmIIprZhjDRPue5ZzsL4WXWw+t4Q/qa62oBRIqGZmPoADZRABO63cdp6IR5jg69za8ztDBVSYBM7iSF3bH5MfNgna0DfuDeY1Op8znMhfpCWR7kjNG+J3gAShRXYslVCHU3j47DrdcZM1btHEi484AuKaYmQCoiQnZk3xHdITtfecOOcwarOxGWiiMDz9Aq0YV2usitOMpsuDC7Dw39TlK4/y1IQ38pTRBhx6zNRLRnPQb+IIKfAbbHg82IIWBVL+EIXrne63rwNxsJGNo7WQRwuxc+MuauT26qUKCztg= 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 Thu, Feb 9, 2023 at 11:13 AM Suren Baghdasaryan wrote: > > On Thu, Feb 9, 2023 at 10:46 AM Eric Biggers wrote: > > > > On Thu, Feb 09, 2023 at 09:09:03AM -0800, Suren Baghdasaryan wrote: > > > On Thu, Feb 2, 2023 at 1:11 PM Suren Baghdasaryan wrote: > > > > > > > > On Wed, Feb 1, 2023 at 8:56 PM Eric Biggers wrote: > > > > > > > > > > On Wed, Feb 01, 2023 at 07:00:23PM -0800, Munehisa Kamata wrote: > > > > > > diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c > > > > > > index 8ac8b81bfee6..6e66c15f6450 100644 > > > > > > --- a/kernel/sched/psi.c > > > > > > +++ b/kernel/sched/psi.c > > > > > > @@ -1343,10 +1343,11 @@ void psi_trigger_destroy(struct psi_trigger *t) > > > > > > > > > > > > group = t->group; > > > > > > /* > > > > > > - * Wakeup waiters to stop polling. Can happen if cgroup is deleted > > > > > > - * from under a polling process. > > > > > > + * Wakeup waiters to stop polling and clear the queue to prevent it from > > > > > > + * being accessed later. Can happen if cgroup is deleted from under a > > > > > > + * polling process otherwise. > > > > > > */ > > > > > > - wake_up_interruptible(&t->event_wait); > > > > > > + wake_up_pollfree(&t->event_wait); > > > > > > > > > > > > mutex_lock(&group->trigger_lock); > > > > > > > > > > wake_up_pollfree() should only be used in extremely rare cases. Why can't the > > > > > lifetime of the waitqueue be fixed instead? > > > > > > > > waitqueue lifetime in this case is linked to cgroup_file_release(), > > > > which seems appropriate to me here. Unfortunately > > > > cgroup_file_release() is not directly linked to the file's lifetime. > > > > For more details see: > > > > https://lore.kernel.org/all/CAJuCfpFZ3B4530TgsSHqp5F_gwfrDujwRYewKReJru==MdEHQg@mail.gmail.com/#t > > > > . > > > > So, if we want to fix the lifetime of the waitqueue, we would have to > > > > tie cgroup_file_release() to the fput() somehow. IOW, the fix would > > > > have to be done at the cgroups or higher (kernfs?) layer. > > > > > > Hi Eric, > > > Do you still object to using wake_up_pollfree() for this case? > > > Changing higher levels to make cgroup_file_release() be tied to fput() > > > would be ideal but I think that would be a big change for this one > > > case. If you agree I'll Ack this patch. > > > Thanks, > > > Suren. > > > > > > > I haven't read the code closely in this case. I'm just letting you know that > > wake_up_pollfree() is very much a last-resort option for when the waitqueue > > lifetime can't be fixed. > > Got it. Thanks for the warning. > I think it can be fixed but the right fix would require a sizable > higher level refactoring which might be more justifiable if we have > more such cases in the future. > > > So if you want to use wake_up_pollfree(), you need to > > explain why no other fix is possible. For example maybe the UAPI depends on the > > waitqueue having a nonstandard lifetime. > > I think the changelog should explain that the waitqueue lifetime in > cases of non-root cgroups is tied to cgroup_file_release() callback, > which in turn is not tied to file's lifetime. That's the reason for > waitqueue and the file having different lifecycles. Would that suffice > as the justification? Ok, in the absence of objections, I would suggest resending this patch with the changelog including details about waitqueue lifetime and reasons wake_up_pollfree() is required here. Munehisa, feel free to reuse https://lore.kernel.org/all/CAJuCfpFZ3B4530TgsSHqp5F_gwfrDujwRYewKReJru==MdEHQg@mail.gmail.com/#t if you find it useful. Thanks, Suren. > Again, I'm not saying that no other fix is possible, but that the > right fix would be much more complex. > Thanks, > Suren. > > > > > - Eric