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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A5702C433FE for ; Tue, 11 Jan 2022 23:37:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346876AbiAKXhx (ORCPT ); Tue, 11 Jan 2022 18:37:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346867AbiAKXht (ORCPT ); Tue, 11 Jan 2022 18:37:49 -0500 Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30222C061751 for ; Tue, 11 Jan 2022 15:37:49 -0800 (PST) Received: by mail-yb1-xb29.google.com with SMTP id d7so1532837ybo.5 for ; Tue, 11 Jan 2022 15:37:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y4s8SWu6oC0jBwQaJEtDME4KEjn8XnX1dHweILOZNLw=; b=tSyF53acF27OqNgvWqbVMB4rrrAYr/G3ysoCmjIQ/DaL271SFRSP9RaoVkMOYsWjkG ZAh/G8g1ZVadBZsGFH5qPak4LK4vm+smNDxvyfLbi93tZ18mFzO1F2hDPQMHLX5dBIsr 5d1kMzt3sJdE7pgOingjfcwZ+mEVWyW1qXaowK0H4IRT5I4jBEhjI3+h7nt/bjknrGED gqlZPxyPkgn1taVWcOXcDy1aihgFYTIfS9B5uiZln7zNU/5M1LElC18H5MoMQC1Z6Jfb serIeUcjt6cll+oqIu7J1g6Uh0ILigdyCTsbA81cViGAUm+Yvdjjz6KDrxShdp6XzC1I hODQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=y4s8SWu6oC0jBwQaJEtDME4KEjn8XnX1dHweILOZNLw=; b=ve7pn0W11u4nHS5L41213FUoR9gl9Gprdawm5X1qoR4Bnh9HvqRYoFjr0D9yemBv+Q zTq/uVwoqdC+Fb9jDRUm8SBWJNjIQRmrpDyVszNYpDLY+F6ZRapxcc4fzEmDUZqeI1FF LtNSm5dPiGoRYmDa8l6om6O7EFX7Y2IZaiiXUFHE+buEp3tcBAh31hYGAN2v3upn1mzA z0iZKRIPA0Mhw9hxvDJZoYfM/FLCIsMSpsZ+aP7lwXYISY8ecj7AN1iqW5CKzC9qyLRU BB2+yO1bv4XWZUTVKvlG+8rWLuMT/CXKMKT+z722ftVIPZPwIGtLySYAoUpXFpNtff/Y EjBQ== X-Gm-Message-State: AOAM533U1pyUw9CiE/heQdfpTNkXcHKK5JA7mvEvkOLLRN8poZQPkgM/ DpRJtbsUJfMLun22OGaK3UwIVj8SGzG3N99Syt0CnA== X-Google-Smtp-Source: ABdhPJyqLXKJ6gdjLMc9e2tpdFFZizV6Y14c+znv0Z5ZDaav7clivseLS2LsVXah/dHpL9Qrebnu61t1tKCpI8Oex8w= X-Received: by 2002:a25:c3c4:: with SMTP id t187mr8315382ybf.634.1641944268131; Tue, 11 Jan 2022 15:37:48 -0800 (PST) MIME-Version: 1.0 References: <20220111071212.1210124-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 11 Jan 2022 15:37:37 -0800 Message-ID: Subject: Re: [PATCH v2 1/1] psi: Fix uaf issue when psi trigger is destroyed while being polled To: Linus Torvalds Cc: Eric Biggers , Johannes Weiner , Tejun Heo , Zefan Li , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Jonathan Corbet , "open list:DOCUMENTATION" , Linux Kernel Mailing List , Cgroups , stable , Android Kernel Team , syzbot Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 11, 2022 at 12:15 PM Linus Torvalds wrote: > > On Tue, Jan 11, 2022 at 11:41 AM Eric Biggers wrote: > > > > This is yet another case of "one time init". > > Ehh. It's somewhat debatable. > > For a flag that sets a value once, the rules are somewhat different. > In that case, people may simply not care about memory ordering at all, > because all they care about is the actual flag value, and - thanks to > the one-time behavior - basically whether some transition had happened > or not. That's not all that unusual. > > But when you fetch a pointer, things are at least conceptually > slightly different. > > Of course, you may use the existence of the pointer itself as a flag > (ie just a "NULL or not"), in which case it's the same as any other > one-time flag thing. > > But if you use it to dereference something, then _by_definition_ > you're not just fetching a one-time flag - even if the pointer is only > set once. At that point, at a minimum, you require that that thing has > been initialized. > > Now, it's then absolutely true that the stuff behind the pointer may > then have other reasons not to care about memory ordering again, and > you may be able to avoid memory ordering even then. If you're just > switching the pointer around between different objects that has been > statically allocated and initialized, then there is no memory ordering > required, for example. You might be back to the "I just want one or > the other of these two pointers". > > But if you have something that was initialized before the pointer was > assigned, you really do hit the problem we had on alpha, where even if > you order the pointer write side accesses, the dereferencing of the > pointer may not be ordered on the read side. > > Now, alpha is basically dead, and we probably don't really care. Even > on alpha, the whole "data dependency isn't a memory ordering" is > almost impossible to trigger. > > And in fact, to avoid too much pain we ended up saying "screw alpha" > and added a memory barrier to READ_ONCE(), so it turns out that > smp_store_release -> READ_ONCE() does work because we just couldn't be > bothered to try something more proper. > > So yeah, READ_ONCE() ends up making the "access through a pointer" > thing safe, but that's less of a "it should be safe" and more of a "we > can't waste time dealing with braindamage on platforms that don't > matter". > > In general, I think the rule should be that READ_ONCE() is for things > that simply don't care about memory ordering at all (or do whatever > ordering they want explicitly). And yes, one such very common case is > the "one-way flag" where once a certain state has been reached, it's > idempotent. > > Of course, then we have the fact that READ_ONCE() can be more > efficient than "smp_load_acquire()" on some platforms, so if something > is *hugely* performance-critical, you might use READ_ONCE() even if > it's not really technically the right thing. > > So it's complicated. > > A lot of READ_ONCE() users exist just for historical reasons because > they predated smp_store_release/smp_load_acquire. They may well have > been using ACCESS_ONCE() long ago. > > And some are there because it's a very critical piece of code, and > it's very intentional. > > But if you don't have some huge reasons, I really would prefer people > use "smp_store_release -> smp_load_acquire" as a very clear "handoff" > event. Posted v3 with smp_store_release/smp_load_acquire: https://lore.kernel.org/all/20220111232309.1786347-1-surenb@google.com Thanks! > > Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suren Baghdasaryan Subject: Re: [PATCH v2 1/1] psi: Fix uaf issue when psi trigger is destroyed while being polled Date: Tue, 11 Jan 2022 15:37:37 -0800 Message-ID: References: <20220111071212.1210124-1-surenb@google.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y4s8SWu6oC0jBwQaJEtDME4KEjn8XnX1dHweILOZNLw=; b=tSyF53acF27OqNgvWqbVMB4rrrAYr/G3ysoCmjIQ/DaL271SFRSP9RaoVkMOYsWjkG ZAh/G8g1ZVadBZsGFH5qPak4LK4vm+smNDxvyfLbi93tZ18mFzO1F2hDPQMHLX5dBIsr 5d1kMzt3sJdE7pgOingjfcwZ+mEVWyW1qXaowK0H4IRT5I4jBEhjI3+h7nt/bjknrGED gqlZPxyPkgn1taVWcOXcDy1aihgFYTIfS9B5uiZln7zNU/5M1LElC18H5MoMQC1Z6Jfb serIeUcjt6cll+oqIu7J1g6Uh0ILigdyCTsbA81cViGAUm+Yvdjjz6KDrxShdp6XzC1I hODQ== In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Linus Torvalds Cc: Eric Biggers , Johannes Weiner , Tejun Heo , Zefan Li , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Jonathan Corbet , "open list:DOCUMENTATION" , Linux Kernel Mailing List , Cgroups , stable , Android Kernel Team , syzbot On Tue, Jan 11, 2022 at 12:15 PM Linus Torvalds wrote: > > On Tue, Jan 11, 2022 at 11:41 AM Eric Biggers wrote: > > > > This is yet another case of "one time init". > > Ehh. It's somewhat debatable. > > For a flag that sets a value once, the rules are somewhat different. > In that case, people may simply not care about memory ordering at all, > because all they care about is the actual flag value, and - thanks to > the one-time behavior - basically whether some transition had happened > or not. That's not all that unusual. > > But when you fetch a pointer, things are at least conceptually > slightly different. > > Of course, you may use the existence of the pointer itself as a flag > (ie just a "NULL or not"), in which case it's the same as any other > one-time flag thing. > > But if you use it to dereference something, then _by_definition_ > you're not just fetching a one-time flag - even if the pointer is only > set once. At that point, at a minimum, you require that that thing has > been initialized. > > Now, it's then absolutely true that the stuff behind the pointer may > then have other reasons not to care about memory ordering again, and > you may be able to avoid memory ordering even then. If you're just > switching the pointer around between different objects that has been > statically allocated and initialized, then there is no memory ordering > required, for example. You might be back to the "I just want one or > the other of these two pointers". > > But if you have something that was initialized before the pointer was > assigned, you really do hit the problem we had on alpha, where even if > you order the pointer write side accesses, the dereferencing of the > pointer may not be ordered on the read side. > > Now, alpha is basically dead, and we probably don't really care. Even > on alpha, the whole "data dependency isn't a memory ordering" is > almost impossible to trigger. > > And in fact, to avoid too much pain we ended up saying "screw alpha" > and added a memory barrier to READ_ONCE(), so it turns out that > smp_store_release -> READ_ONCE() does work because we just couldn't be > bothered to try something more proper. > > So yeah, READ_ONCE() ends up making the "access through a pointer" > thing safe, but that's less of a "it should be safe" and more of a "we > can't waste time dealing with braindamage on platforms that don't > matter". > > In general, I think the rule should be that READ_ONCE() is for things > that simply don't care about memory ordering at all (or do whatever > ordering they want explicitly). And yes, one such very common case is > the "one-way flag" where once a certain state has been reached, it's > idempotent. > > Of course, then we have the fact that READ_ONCE() can be more > efficient than "smp_load_acquire()" on some platforms, so if something > is *hugely* performance-critical, you might use READ_ONCE() even if > it's not really technically the right thing. > > So it's complicated. > > A lot of READ_ONCE() users exist just for historical reasons because > they predated smp_store_release/smp_load_acquire. They may well have > been using ACCESS_ONCE() long ago. > > And some are there because it's a very critical piece of code, and > it's very intentional. > > But if you don't have some huge reasons, I really would prefer people > use "smp_store_release -> smp_load_acquire" as a very clear "handoff" > event. Posted v3 with smp_store_release/smp_load_acquire: https://lore.kernel.org/all/20220111232309.1786347-1-surenb@google.com Thanks! > > Linus