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 BC4D1C636D6 for ; Thu, 9 Feb 2023 19:13:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 32CCB6B0071; Thu, 9 Feb 2023 14:13:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2DD466B0072; Thu, 9 Feb 2023 14:13:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1A4466B0078; Thu, 9 Feb 2023 14:13:17 -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 0B0786B0071 for ; Thu, 9 Feb 2023 14:13:17 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D0040AAB43 for ; Thu, 9 Feb 2023 19:13:16 +0000 (UTC) X-FDA: 80448701592.24.AECEC73 Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) by imf16.hostedemail.com (Postfix) with ESMTP id 0E5AB180015 for ; Thu, 9 Feb 2023 19:13:14 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=cUJXptVK; spf=pass (imf16.hostedemail.com: domain of surenb@google.com designates 209.85.219.171 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=1675969995; 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=0WFna3RnbyIN5xpLlcRG0QF1tzU7bO3nMdV/kLugpFI=; b=3xp9AFETVxBMlo0mvwypQUhDEc5MqiFRIGbOidruQXSBTxEu/zAy4GM9ewXXtc4S9FNtf6 25vpP8gwODzrFyOOczxZ8hyzGN/6Nm8K75xrfZiapkGyIA64xdt0vryPQsf444SLu+BMCu HARYFM4+TAjUU8Q4IR6LTiH9IkdPsUk= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=cUJXptVK; spf=pass (imf16.hostedemail.com: domain of surenb@google.com designates 209.85.219.171 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=1675969995; a=rsa-sha256; cv=none; b=i6fypzMGxTgpGkVJ1FxCYJPueMEi0Y6FzI2Cp3lswJmis38R2uwce6ItcS5hoFCai3p5HW AB0XPDJvAzHA6x/eKu96GpYJCs4wZ0kQ9D6Pz+ZI6H8jUlWp4MflSOWo1krMxwQ4RwxsBb bwQkj42v3gwPhTUnsXIZOZIeYx50rQs= Received: by mail-yb1-f171.google.com with SMTP id 139so2176404ybe.3 for ; Thu, 09 Feb 2023 11:13:14 -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=0WFna3RnbyIN5xpLlcRG0QF1tzU7bO3nMdV/kLugpFI=; b=cUJXptVKcev0jWNG0fpaHealHgyPAha/CVW/u3EfdI9Z4La30pZSwbtwXbI0fr9EAd Wx6XDYq+Uat9LNVQqGVixbDNIdj0p8k7zciauDxshDBtuUMkIK6cklsAzKyQ49WwFR6N SUr/o6KVjqez8JGDYRtr84g7HPedYdwZbpq/cct/A+jYgpCPSX0cFECW2Vtonjyulikj I2rYTgLXVwaOwQAFs2cy2Ojn1lIbDM7DjMpP1IekpDtgffe+JKmWxQNWiMMfNRT0K/eT OR1+beltKfbp9ZB/65Osl6n6XsBKvgPiUlSZ9cPFcxPAtFiqzzxs3AZKR86opgSZR1Vg L87w== 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=0WFna3RnbyIN5xpLlcRG0QF1tzU7bO3nMdV/kLugpFI=; b=v45qG9Z5kq0FFU2qXMjPJ0kkZV+VUePddxVmy/Hn5x2iB13ZTRIdjLdSAxXoTuZuFq /W+lS9JBi3ugRwxp2NRmI3RTJ1XYf8YKcePljKZhTXlnrWvYf9e/Vxs6Dk8PqEcX1U+9 TeGeKjWyMn8euEI1opB43bDjtKML5YZgjxXLicTjUlnzxve/aqIJKzqGWdsuIuiKPtAq gEHOQVcTP8yRvc5S0fXFfqVnu6c8yqOMOn6X06mE5NqAHwBRc3ErIXcWLFGGenpd3S74 x4h51Uhyo1owWQvsOhrKwdFrfH+yQFwEAG7TEe/UqWDEKitap25LXamt3jXkkmrkSsOc vIWA== X-Gm-Message-State: AO0yUKW4NBXRSf6Mx8hnA6pMt/hr0DIk4MBqq24lvwgJaB/Ux8oc7e1H QsxiQJ4Ndf9NPG5aOZkNKI/rq2oajSelak50kGeGMQ== X-Google-Smtp-Source: AK7set/D/XHV8uJf1p3K2MyhQKXuCrrMpzLjcB2ACdTq1kWeMTM7NQl1s3QPQPiKOSlGvN1jVScuRJJcqmjPXjwTPRY= X-Received: by 2002:a05:6902:510:b0:8b2:bb9a:a9d6 with SMTP id x16-20020a056902051000b008b2bb9aa9d6mr897129ybs.431.1675969993962; Thu, 09 Feb 2023 11:13:13 -0800 (PST) MIME-Version: 1.0 References: <20230202030023.1847084-1-kamatam@amazon.com> In-Reply-To: From: Suren Baghdasaryan Date: Thu, 9 Feb 2023 11:13:02 -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-Rspamd-Queue-Id: 0E5AB180015 X-Stat-Signature: 11gkmw4y8kwsin84yjezp91o5b8finw4 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1675969994-909746 X-HE-Meta: U2FsdGVkX1/E+G1nPMzAQSkIPJBGLmP/v3q4G2Ksx7MqPWU77edErn7Ox49UvQnjApjzzcVD289EiEt19jOhOTghwe9jHCex8gezYdbfrE6Y7+W4zl33oI+7a3sI0IyGxynqZppPGPPArkwvQG7CbWLz60tb6N2aVLNzhtQJkZO2bQs9gls3T6CxHLen9wLbja3sCqgpfAcjmOsaMRoAaz+vAor4xil3/bAtTUFoWPEuSBO09T2gjm9zliGBP6liNd0FMjnD2SBId2kOeUs4MkZ72rie+XuvvVa7QRKDC0dWqeZ6IcdAxswkpaKD9QcvMVvguq8N7YZC8T14mtIFYRYQU6/uZ1cKpOTWbJe2Re+nwJy3gJ1twpPaYjHW1E3HgZqR+cR1et9FKtnVrxa/DeMzB8N5oYu9/Iw0Ec1xkl9XS82BH695QqFc2tXi24BHb9fJO3UJBkcmj4HBtH0uPAXwQyGQK11IErbe0F+68VlzeQ00TBBKmcgupxQ+Zjv7E0evO2l81W5JmTQm+XfZjJoWA0asrwYhmDQbI50ej2YlQApMPqaectPo4mPSBk3fE8lLnBj26Gq4Alr3ThllFcPI0zwq86PotMiTV3P5Lnosl2zbBfKjm0ADfTrUNqbCaap1321vQnzjQzkalEUZpygT8kGO70G9r6B4uWiI4vr1JB1mc9M+1N7HVL823nfoAesUvqxKMp5UFrPYgNT8cQ7rXFOCqJB5BiGuU0aKSETUCcbiyulrV6XsAG/clLJwENZIbmG0m3O2gkhf6WSL8NBFwpi3wvt/217DgbdKnxJf3tIyxyqFg5E03W5JYrfG8zOkEDI0p0O3cGsHauvTeMp0YDwLo50vgCM3oAIr27KMxVb5q11fCuaHsG78g46abgEi28tTUqJmqt5Mx7RRgwSxYJ0Ub9NVNKWVavW8P11LjcFpyYS42xPQ7mujFW2uIAdjlMQtFb01HEPtylO 62eCTfeQ RFFNYZZ2h7UGWxemlsp8F5yA95EGXKCPezFHPhjb/8/x669ex9USqQGv8awORC9Xu/W33x+fmFKExcQ819sv5RgY7kod/kwG+KqDfZJo87S9X51JIO4uj1xhKtNFXO/kMXXtLSjbD5ezgR4FBseexlobGSyUdBZmMYe5+8KCVfLYtaTuT3AFAaDUNfDCnz61EWQF0pcudJuu/q3RRLrHM+RRgHHLgbNQ7lnn9YlhfnlyUeC3RpgBEsUZlQxye9IltaKMMkY+kAqCe3HrnRIRbfPXHSt87oEgQLFpd2Fho/DggKeUShUwXOb0bPCCYgFfwK9NVjwnYOVWL/CRm+SY+iQ6VZKz3kx5mwHcq5zQ5czuOdEIDHLpv2I0iCehn8uRcUrTlubHGZOdQgcg= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000013, 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 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? Again, I'm not saying that no other fix is possible, but that the right fix would be much more complex. Thanks, Suren. > > - Eric