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 X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BB171C43142 for ; Tue, 31 Jul 2018 18:52:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 60E2F20844 for ; Tue, 31 Jul 2018 18:52:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="ZsBjdhfo" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 60E2F20844 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732106AbeGaUdu (ORCPT ); Tue, 31 Jul 2018 16:33:50 -0400 Received: from mail-it0-f65.google.com ([209.85.214.65]:52447 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729772AbeGaUdu (ORCPT ); Tue, 31 Jul 2018 16:33:50 -0400 Received: by mail-it0-f65.google.com with SMTP id d9-v6so6009410itf.2; Tue, 31 Jul 2018 11:52:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=b5s/1w6NfwWmPUI51XE91GmZmvzu/XMZo0AQLSGNCm4=; b=ZsBjdhfojpz5SnSKKCC08yr/0rlghwKx7KXUR8oPhEI3grzNZ6DIwH/s3YdEh8KM3C /8IZ3I4BzVexqushZv2yFcXmhXb2CxbcrEHfdczVAmEivmu+3LpZ5ynVb7DRXCgQOStX S5iAN5fbiBQ+3fC22qHy+Hl4lJPosu25IL1oQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=b5s/1w6NfwWmPUI51XE91GmZmvzu/XMZo0AQLSGNCm4=; b=LlpKV/kaaM+Gkf40M0v3X84cPVyysNcVtwF2+bCHc+wcCtMHl5WuZNHmWWsy8FBU0n dLSD2C46sZcYlpjxwtBy3/w6g3m6uki1B5+BXpyfW+2eNoD6fYGYzMZEkvL+XS6LBQNq Iej8oWflX6ubY92MB7s0FN21DAR/fZX3Qh9N5FlSFEXzWXWN4JdK6Bvk4STn9MdP4LNa BuEEV6gH79eQuebXeEkVKRbXZggmhH25hQgirMFyfFcXnk1VugumfU2naIxQV76NDyMN HcH+l8Iwy6aj5vGU8VK/EhqBR5lOr+tkAJa6qakse1eXvKOEYD48pDuRdKBhikL4fUkz cPuw== X-Gm-Message-State: AOUpUlFz405yaXnAVN8Rx5BZUMRxS9C4QP9BQEH/Ac3cDxsUTlsYMBcd OtFjPua08W4PtH7vO7GDwWRtYiG67MBoRs4MUk0= X-Google-Smtp-Source: AAOMgpe1tqMGrrLHqYxd8jWmLxtdEqk6P5NdsLgoCHiV8NgQM9x91oCFuEdwArNae+7yNhago58krSLOFraQRQOGBE0= X-Received: by 2002:a24:5002:: with SMTP id m2-v6mr827060itb.16.1533063130555; Tue, 31 Jul 2018 11:52:10 -0700 (PDT) MIME-Version: 1.0 References: <01000164f169bc6b-c73a8353-d7d9-47ec-a782-90aadcb86bfb-000000@email.amazonses.com> In-Reply-To: From: Linus Torvalds Date: Tue, 31 Jul 2018 11:51:59 -0700 Message-ID: Subject: Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation) To: Christoph Lameter Cc: Andrey Ryabinin , "Theodore Ts'o" , Jan Kara , linux-ext4@vger.kernel.org, Greg Kroah-Hartman , Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , David Miller , NetFilter , coreteam@netfilter.org, Network Development , gerrit@erg.abdn.ac.uk, dccp@vger.kernel.org, Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Dave Airlie , intel-gfx , DRI , Eric Dumazet , Alexey Kuznetsov , Hideaki YOSHIFUJI , Ursula Braun , linux-s390 , Linux Kernel Mailing List , Dmitry Vyukov , Andrew Morton , linux-mm , Andrey Konovalov Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 31, 2018 at 10:49 AM Linus Torvalds wrote: > > So the re-use might initialize the fields lazily, not necessarily using a ctor. In particular, the pattern that nf_conntrack uses looks like it is safe. If you have a well-defined refcount, and use "atomic_inc_not_zero()" to guard the speculative RCU access section, and use "atomic_dec_and_test()" in the freeing section, then you should be safe wrt new allocations. If you have a completely new allocation that has "random stale content", you know that it cannot be on the RCU list, so there is no speculative access that can ever see that random content. So the only case you need to worry about is a re-use allocation, and you know that the refcount will start out as zero even if you don't have a constructor. So you can think of the refcount itself as always having a zero constructor, *BUT* you need to be careful with ordering. In particular, whoever does the allocation needs to then set the refcount to a non-zero value *after* it has initialized all the other fields. And in particular, it needs to make sure that it uses the proper memory ordering to do so. And in this case, we have static struct nf_conn * __nf_conntrack_alloc(struct net *net, { ... atomic_set(&ct->ct_general.use, 0); which is a no-op for the re-use case (whether racing or not, since any "inc_not_zero" users won't touch it), but initializes it to zero for the "completely new object" case. And then, the thing that actually exposes it to the speculative walkers does: int nf_conntrack_hash_check_insert(struct nf_conn *ct) { ... smp_wmb(); /* The caller holds a reference to this object */ atomic_set(&ct->ct_general.use, 2); which means that it stays as zero until everything is actually set up, and then the optimistic walker can use the other fields (including spinlocks etc) to verify that it's actually the right thing. The smp_wmb() means that the previous initialization really will be visible before the object is visible. Side note: on some architectures it might help to make that "smp_wmb -> atomic_set()" sequence be am "smp_store_release()" instead. Doesn't matter on x86, but might matter on arm64. NOTE! One thing to be very worried about is that re-initializing whatever RCU lists means that now the RCU walker may be walking on the wrong list so the walker may do the right thing for this particular entry, but it may miss walking *other* entries. So then you can get spurious lookup failures, because the RCU walker never walked all the way to the end of the right list. That ends up being a much more subtle bug. But the nf_conntrack case seems to get that right too, see the restart in ____nf_conntrack_find(). So I don't see anything wrong in nf_conntrack. But yes, using SLAB_TYPESAFE_BY_RCU is very very subtle. But most of the subtleties have nothing to do with having a constructor, they are about those "make sure memory ordering wrt refcount is right" and "restart speculative RCU walk" issues that actually happen regardless of having a constructor or not. Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation) Date: Tue, 31 Jul 2018 11:51:59 -0700 Message-ID: References: <01000164f169bc6b-c73a8353-d7d9-47ec-a782-90aadcb86bfb-000000@email.amazonses.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: Dave Airlie , DRI , linux-mm , Eric Dumazet , Network Development , gerrit@erg.abdn.ac.uk, dccp@vger.kernel.org, coreteam@netfilter.org, Jozsef Kadlecsik , Andrey Ryabinin , linux-ext4@vger.kernel.org, Alexey Kuznetsov , Pablo Neira Ayuso , linux-s390 , Andrey Konovalov , intel-gfx , Ursula Braun , Rodrigo Vivi , Dmitry Vyukov , Theodore Ts'o , Hideaki YOSHIFUJI , Greg Kroah-Hartman , Florian Westphal , Linux Kernel Mailing List Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" List-Id: netdev.vger.kernel.org T24gVHVlLCBKdWwgMzEsIDIwMTggYXQgMTA6NDkgQU0gTGludXMgVG9ydmFsZHMKPHRvcnZhbGRz QGxpbnV4LWZvdW5kYXRpb24ub3JnPiB3cm90ZToKPgo+IFNvIHRoZSByZS11c2UgbWlnaHQgaW5p dGlhbGl6ZSB0aGUgZmllbGRzIGxhemlseSwgbm90IG5lY2Vzc2FyaWx5IHVzaW5nIGEgY3Rvci4K CkluIHBhcnRpY3VsYXIsIHRoZSBwYXR0ZXJuIHRoYXQgbmZfY29ubnRyYWNrIHVzZXMgbG9va3Mg bGlrZSBpdCBpcyBzYWZlLgoKSWYgeW91IGhhdmUgYSB3ZWxsLWRlZmluZWQgcmVmY291bnQsIGFu ZCB1c2UgImF0b21pY19pbmNfbm90X3plcm8oKSIKdG8gZ3VhcmQgdGhlIHNwZWN1bGF0aXZlIFJD VSBhY2Nlc3Mgc2VjdGlvbiwgYW5kIHVzZQoiYXRvbWljX2RlY19hbmRfdGVzdCgpIiBpbiB0aGUg ZnJlZWluZyBzZWN0aW9uLCB0aGVuIHlvdSBzaG91bGQgYmUKc2FmZSB3cnQgbmV3IGFsbG9jYXRp b25zLgoKSWYgeW91IGhhdmUgYSBjb21wbGV0ZWx5IG5ldyBhbGxvY2F0aW9uIHRoYXQgaGFzICJy YW5kb20gc3RhbGUKY29udGVudCIsIHlvdSBrbm93IHRoYXQgaXQgY2Fubm90IGJlIG9uIHRoZSBS Q1UgbGlzdCwgc28gdGhlcmUgaXMgbm8Kc3BlY3VsYXRpdmUgYWNjZXNzIHRoYXQgY2FuIGV2ZXIg c2VlIHRoYXQgcmFuZG9tIGNvbnRlbnQuCgpTbyB0aGUgb25seSBjYXNlIHlvdSBuZWVkIHRvIHdv cnJ5IGFib3V0IGlzIGEgcmUtdXNlIGFsbG9jYXRpb24sIGFuZAp5b3Uga25vdyB0aGF0IHRoZSBy ZWZjb3VudCB3aWxsIHN0YXJ0IG91dCBhcyB6ZXJvIGV2ZW4gaWYgeW91IGRvbid0CmhhdmUgYSBj b25zdHJ1Y3Rvci4KClNvIHlvdSBjYW4gdGhpbmsgb2YgdGhlIHJlZmNvdW50IGl0c2VsZiBhcyBh bHdheXMgaGF2aW5nIGEgemVybwpjb25zdHJ1Y3RvciwgKkJVVCogeW91IG5lZWQgdG8gYmUgY2Fy ZWZ1bCB3aXRoIG9yZGVyaW5nLgoKSW4gcGFydGljdWxhciwgd2hvZXZlciBkb2VzIHRoZSBhbGxv Y2F0aW9uIG5lZWRzIHRvIHRoZW4gc2V0IHRoZQpyZWZjb3VudCB0byBhIG5vbi16ZXJvIHZhbHVl ICphZnRlciogaXQgaGFzIGluaXRpYWxpemVkIGFsbCB0aGUgb3RoZXIKZmllbGRzLiBBbmQgaW4g cGFydGljdWxhciwgaXQgbmVlZHMgdG8gbWFrZSBzdXJlIHRoYXQgaXQgdXNlcyB0aGUKcHJvcGVy IG1lbW9yeSBvcmRlcmluZyB0byBkbyBzby4KCkFuZCBpbiB0aGlzIGNhc2UsIHdlIGhhdmUKCiAg c3RhdGljIHN0cnVjdCBuZl9jb25uICoKICBfX25mX2Nvbm50cmFja19hbGxvYyhzdHJ1Y3QgbmV0 ICpuZXQsCiAgewogICAgICAgIC4uLgogICAgICAgIGF0b21pY19zZXQoJmN0LT5jdF9nZW5lcmFs LnVzZSwgMCk7Cgp3aGljaCBpcyBhIG5vLW9wIGZvciB0aGUgcmUtdXNlIGNhc2UgKHdoZXRoZXIg cmFjaW5nIG9yIG5vdCwgc2luY2UgYW55CiJpbmNfbm90X3plcm8iIHVzZXJzIHdvbid0IHRvdWNo IGl0KSwgYnV0IGluaXRpYWxpemVzIGl0IHRvIHplcm8gZm9yCnRoZSAiY29tcGxldGVseSBuZXcg b2JqZWN0IiBjYXNlLgoKQW5kIHRoZW4sIHRoZSB0aGluZyB0aGF0IGFjdHVhbGx5IGV4cG9zZXMg aXQgdG8gdGhlIHNwZWN1bGF0aXZlIHdhbGtlcnMgZG9lczoKCiAgaW50CiAgbmZfY29ubnRyYWNr X2hhc2hfY2hlY2tfaW5zZXJ0KHN0cnVjdCBuZl9jb25uICpjdCkKICB7CiAgICAgICAgLi4uCiAg ICAgICAgc21wX3dtYigpOwogICAgICAgIC8qIFRoZSBjYWxsZXIgaG9sZHMgYSByZWZlcmVuY2Ug dG8gdGhpcyBvYmplY3QgKi8KICAgICAgICBhdG9taWNfc2V0KCZjdC0+Y3RfZ2VuZXJhbC51c2Us IDIpOwoKd2hpY2ggbWVhbnMgdGhhdCBpdCBzdGF5cyBhcyB6ZXJvIHVudGlsIGV2ZXJ5dGhpbmcg aXMgYWN0dWFsbHkgc2V0IHVwLAphbmQgdGhlbiB0aGUgb3B0aW1pc3RpYyB3YWxrZXIgY2FuIHVz ZSB0aGUgb3RoZXIgZmllbGRzIChpbmNsdWRpbmcKc3BpbmxvY2tzIGV0YykgdG8gdmVyaWZ5IHRo YXQgaXQncyBhY3R1YWxseSB0aGUgcmlnaHQgdGhpbmcuIFRoZQpzbXBfd21iKCkgbWVhbnMgdGhh dCB0aGUgcHJldmlvdXMgaW5pdGlhbGl6YXRpb24gcmVhbGx5IHdpbGwgYmUKdmlzaWJsZSBiZWZv cmUgdGhlIG9iamVjdCBpcyB2aXNpYmxlLgoKU2lkZSBub3RlOiBvbiBzb21lIGFyY2hpdGVjdHVy ZXMgaXQgbWlnaHQgaGVscCB0byBtYWtlIHRoYXQgInNtcF93bWIKLT4gYXRvbWljX3NldCgpIiBz ZXF1ZW5jZSBiZSBhbSAic21wX3N0b3JlX3JlbGVhc2UoKSIgaW5zdGVhZC4gRG9lc24ndAptYXR0 ZXIgb24geDg2LCBidXQgbWlnaHQgbWF0dGVyIG9uIGFybTY0LgoKTk9URSEgT25lIHRoaW5nIHRv IGJlIHZlcnkgd29ycmllZCBhYm91dCBpcyB0aGF0IHJlLWluaXRpYWxpemluZwp3aGF0ZXZlciBS Q1UgbGlzdHMgbWVhbnMgdGhhdCBub3cgdGhlIFJDVSB3YWxrZXIgbWF5IGJlIHdhbGtpbmcgb24g dGhlCndyb25nIGxpc3Qgc28gdGhlIHdhbGtlciBtYXkgZG8gdGhlIHJpZ2h0IHRoaW5nIGZvciB0 aGlzIHBhcnRpY3VsYXIKZW50cnksIGJ1dCBpdCBtYXkgbWlzcyB3YWxraW5nICpvdGhlciogZW50 cmllcy4gU28gdGhlbiB5b3UgY2FuIGdldApzcHVyaW91cyBsb29rdXAgZmFpbHVyZXMsIGJlY2F1 c2UgdGhlIFJDVSB3YWxrZXIgbmV2ZXIgd2Fsa2VkIGFsbCB0aGUKd2F5IHRvIHRoZSBlbmQgb2Yg dGhlIHJpZ2h0IGxpc3QuIFRoYXQgZW5kcyB1cCBiZWluZyBhIG11Y2ggbW9yZQpzdWJ0bGUgYnVn LgoKQnV0IHRoZSBuZl9jb25udHJhY2sgY2FzZSBzZWVtcyB0byBnZXQgdGhhdCByaWdodCB0b28s IHNlZSB0aGUgcmVzdGFydAppbiBfX19fbmZfY29ubnRyYWNrX2ZpbmQoKS4KClNvIEkgZG9uJ3Qg c2VlIGFueXRoaW5nIHdyb25nIGluIG5mX2Nvbm50cmFjay4KCkJ1dCB5ZXMsIHVzaW5nIFNMQUJf VFlQRVNBRkVfQllfUkNVIGlzIHZlcnkgdmVyeSBzdWJ0bGUuIEJ1dCBtb3N0IG9mCnRoZSBzdWJ0 bGV0aWVzIGhhdmUgbm90aGluZyB0byBkbyB3aXRoIGhhdmluZyBhIGNvbnN0cnVjdG9yLCB0aGV5 IGFyZQphYm91dCB0aG9zZSAibWFrZSBzdXJlIG1lbW9yeSBvcmRlcmluZyB3cnQgcmVmY291bnQg aXMgcmlnaHQiIGFuZAoicmVzdGFydCBzcGVjdWxhdGl2ZSBSQ1Ugd2FsayIgaXNzdWVzIHRoYXQg YWN0dWFsbHkgaGFwcGVuIHJlZ2FyZGxlc3MKb2YgaGF2aW5nIGEgY29uc3RydWN0b3Igb3Igbm90 LgoKICAgICAgICAgICAgICAgICAgTGludXMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation) Date: Tue, 31 Jul 2018 11:51:59 -0700 Message-ID: References: <01000164f169bc6b-c73a8353-d7d9-47ec-a782-90aadcb86bfb-000000@email.amazonses.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" List-Archive: List-Post: To: Christoph Lameter Cc: Dave Airlie , DRI , linux-mm , Eric Dumazet , Network Development , gerrit@erg.abdn.ac.uk, dccp@vger.kernel.org, coreteam@netfilter.org, Jozsef Kadlecsik , Andrey Ryabinin , linux-ext4@vger.kernel.org, Alexey Kuznetsov , Pablo Neira Ayuso , linux-s390 , Andrey Konovalov , intel-gfx , Ursula Braun , Rodrigo Vivi , Dmitry Vyukov , Theodore Ts'o , Hideaki YOSHIFUJI , Greg Kroah-Hartman , Florian Westphal List-ID: T24gVHVlLCBKdWwgMzEsIDIwMTggYXQgMTA6NDkgQU0gTGludXMgVG9ydmFsZHMKPHRvcnZhbGRz QGxpbnV4LWZvdW5kYXRpb24ub3JnPiB3cm90ZToKPgo+IFNvIHRoZSByZS11c2UgbWlnaHQgaW5p dGlhbGl6ZSB0aGUgZmllbGRzIGxhemlseSwgbm90IG5lY2Vzc2FyaWx5IHVzaW5nIGEgY3Rvci4K CkluIHBhcnRpY3VsYXIsIHRoZSBwYXR0ZXJuIHRoYXQgbmZfY29ubnRyYWNrIHVzZXMgbG9va3Mg bGlrZSBpdCBpcyBzYWZlLgoKSWYgeW91IGhhdmUgYSB3ZWxsLWRlZmluZWQgcmVmY291bnQsIGFu ZCB1c2UgImF0b21pY19pbmNfbm90X3plcm8oKSIKdG8gZ3VhcmQgdGhlIHNwZWN1bGF0aXZlIFJD VSBhY2Nlc3Mgc2VjdGlvbiwgYW5kIHVzZQoiYXRvbWljX2RlY19hbmRfdGVzdCgpIiBpbiB0aGUg ZnJlZWluZyBzZWN0aW9uLCB0aGVuIHlvdSBzaG91bGQgYmUKc2FmZSB3cnQgbmV3IGFsbG9jYXRp b25zLgoKSWYgeW91IGhhdmUgYSBjb21wbGV0ZWx5IG5ldyBhbGxvY2F0aW9uIHRoYXQgaGFzICJy YW5kb20gc3RhbGUKY29udGVudCIsIHlvdSBrbm93IHRoYXQgaXQgY2Fubm90IGJlIG9uIHRoZSBS Q1UgbGlzdCwgc28gdGhlcmUgaXMgbm8Kc3BlY3VsYXRpdmUgYWNjZXNzIHRoYXQgY2FuIGV2ZXIg c2VlIHRoYXQgcmFuZG9tIGNvbnRlbnQuCgpTbyB0aGUgb25seSBjYXNlIHlvdSBuZWVkIHRvIHdv cnJ5IGFib3V0IGlzIGEgcmUtdXNlIGFsbG9jYXRpb24sIGFuZAp5b3Uga25vdyB0aGF0IHRoZSBy ZWZjb3VudCB3aWxsIHN0YXJ0IG91dCBhcyB6ZXJvIGV2ZW4gaWYgeW91IGRvbid0CmhhdmUgYSBj b25zdHJ1Y3Rvci4KClNvIHlvdSBjYW4gdGhpbmsgb2YgdGhlIHJlZmNvdW50IGl0c2VsZiBhcyBh bHdheXMgaGF2aW5nIGEgemVybwpjb25zdHJ1Y3RvciwgKkJVVCogeW91IG5lZWQgdG8gYmUgY2Fy ZWZ1bCB3aXRoIG9yZGVyaW5nLgoKSW4gcGFydGljdWxhciwgd2hvZXZlciBkb2VzIHRoZSBhbGxv Y2F0aW9uIG5lZWRzIHRvIHRoZW4gc2V0IHRoZQpyZWZjb3VudCB0byBhIG5vbi16ZXJvIHZhbHVl ICphZnRlciogaXQgaGFzIGluaXRpYWxpemVkIGFsbCB0aGUgb3RoZXIKZmllbGRzLiBBbmQgaW4g cGFydGljdWxhciwgaXQgbmVlZHMgdG8gbWFrZSBzdXJlIHRoYXQgaXQgdXNlcyB0aGUKcHJvcGVy IG1lbW9yeSBvcmRlcmluZyB0byBkbyBzby4KCkFuZCBpbiB0aGlzIGNhc2UsIHdlIGhhdmUKCiAg c3RhdGljIHN0cnVjdCBuZl9jb25uICoKICBfX25mX2Nvbm50cmFja19hbGxvYyhzdHJ1Y3QgbmV0 ICpuZXQsCiAgewogICAgICAgIC4uLgogICAgICAgIGF0b21pY19zZXQoJmN0LT5jdF9nZW5lcmFs LnVzZSwgMCk7Cgp3aGljaCBpcyBhIG5vLW9wIGZvciB0aGUgcmUtdXNlIGNhc2UgKHdoZXRoZXIg cmFjaW5nIG9yIG5vdCwgc2luY2UgYW55CiJpbmNfbm90X3plcm8iIHVzZXJzIHdvbid0IHRvdWNo IGl0KSwgYnV0IGluaXRpYWxpemVzIGl0IHRvIHplcm8gZm9yCnRoZSAiY29tcGxldGVseSBuZXcg b2JqZWN0IiBjYXNlLgoKQW5kIHRoZW4sIHRoZSB0aGluZyB0aGF0IGFjdHVhbGx5IGV4cG9zZXMg aXQgdG8gdGhlIHNwZWN1bGF0aXZlIHdhbGtlcnMgZG9lczoKCiAgaW50CiAgbmZfY29ubnRyYWNr X2hhc2hfY2hlY2tfaW5zZXJ0KHN0cnVjdCBuZl9jb25uICpjdCkKICB7CiAgICAgICAgLi4uCiAg ICAgICAgc21wX3dtYigpOwogICAgICAgIC8qIFRoZSBjYWxsZXIgaG9sZHMgYSByZWZlcmVuY2Ug dG8gdGhpcyBvYmplY3QgKi8KICAgICAgICBhdG9taWNfc2V0KCZjdC0+Y3RfZ2VuZXJhbC51c2Us IDIpOwoKd2hpY2ggbWVhbnMgdGhhdCBpdCBzdGF5cyBhcyB6ZXJvIHVudGlsIGV2ZXJ5dGhpbmcg aXMgYWN0dWFsbHkgc2V0IHVwLAphbmQgdGhlbiB0aGUgb3B0aW1pc3RpYyB3YWxrZXIgY2FuIHVz ZSB0aGUgb3RoZXIgZmllbGRzIChpbmNsdWRpbmcKc3BpbmxvY2tzIGV0YykgdG8gdmVyaWZ5IHRo YXQgaXQncyBhY3R1YWxseSB0aGUgcmlnaHQgdGhpbmcuIFRoZQpzbXBfd21iKCkgbWVhbnMgdGhh dCB0aGUgcHJldmlvdXMgaW5pdGlhbGl6YXRpb24gcmVhbGx5IHdpbGwgYmUKdmlzaWJsZSBiZWZv cmUgdGhlIG9iamVjdCBpcyB2aXNpYmxlLgoKU2lkZSBub3RlOiBvbiBzb21lIGFyY2hpdGVjdHVy ZXMgaXQgbWlnaHQgaGVscCB0byBtYWtlIHRoYXQgInNtcF93bWIKLT4gYXRvbWljX3NldCgpIiBz ZXF1ZW5jZSBiZSBhbSAic21wX3N0b3JlX3JlbGVhc2UoKSIgaW5zdGVhZC4gRG9lc24ndAptYXR0 ZXIgb24geDg2LCBidXQgbWlnaHQgbWF0dGVyIG9uIGFybTY0LgoKTk9URSEgT25lIHRoaW5nIHRv IGJlIHZlcnkgd29ycmllZCBhYm91dCBpcyB0aGF0IHJlLWluaXRpYWxpemluZwp3aGF0ZXZlciBS Q1UgbGlzdHMgbWVhbnMgdGhhdCBub3cgdGhlIFJDVSB3YWxrZXIgbWF5IGJlIHdhbGtpbmcgb24g dGhlCndyb25nIGxpc3Qgc28gdGhlIHdhbGtlciBtYXkgZG8gdGhlIHJpZ2h0IHRoaW5nIGZvciB0 aGlzIHBhcnRpY3VsYXIKZW50cnksIGJ1dCBpdCBtYXkgbWlzcyB3YWxraW5nICpvdGhlciogZW50 cmllcy4gU28gdGhlbiB5b3UgY2FuIGdldApzcHVyaW91cyBsb29rdXAgZmFpbHVyZXMsIGJlY2F1 c2UgdGhlIFJDVSB3YWxrZXIgbmV2ZXIgd2Fsa2VkIGFsbCB0aGUKd2F5IHRvIHRoZSBlbmQgb2Yg dGhlIHJpZ2h0IGxpc3QuIFRoYXQgZW5kcyB1cCBiZWluZyBhIG11Y2ggbW9yZQpzdWJ0bGUgYnVn LgoKQnV0IHRoZSBuZl9jb25udHJhY2sgY2FzZSBzZWVtcyB0byBnZXQgdGhhdCByaWdodCB0b28s IHNlZSB0aGUgcmVzdGFydAppbiBfX19fbmZfY29ubnRyYWNrX2ZpbmQoKS4KClNvIEkgZG9uJ3Qg c2VlIGFueXRoaW5nIHdyb25nIGluIG5mX2Nvbm50cmFjay4KCkJ1dCB5ZXMsIHVzaW5nIFNMQUJf VFlQRVNBRkVfQllfUkNVIGlzIHZlcnkgdmVyeSBzdWJ0bGUuIEJ1dCBtb3N0IG9mCnRoZSBzdWJ0 bGV0aWVzIGhhdmUgbm90aGluZyB0byBkbyB3aXRoIGhhdmluZyBhIGNvbnN0cnVjdG9yLCB0aGV5 IGFyZQphYm91dCB0aG9zZSAibWFrZSBzdXJlIG1lbW9yeSBvcmRlcmluZyB3cnQgcmVmY291bnQg aXMgcmlnaHQiIGFuZAoicmVzdGFydCBzcGVjdWxhdGl2ZSBSQ1Ugd2FsayIgaXNzdWVzIHRoYXQg YWN0dWFsbHkgaGFwcGVuIHJlZ2FyZGxlc3MKb2YgaGF2aW5nIGEgY29uc3RydWN0b3Igb3Igbm90 LgoKICAgICAgICAgICAgICAgICAgTGludXMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation) Date: Tue, 31 Jul 2018 11:51:59 -0700 Message-ID: References: <01000164f169bc6b-c73a8353-d7d9-47ec-a782-90aadcb86bfb-000000@email.amazonses.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Christoph Lameter Cc: Dave Airlie , DRI , linux-mm , Eric Dumazet , Network Development , gerrit@erg.abdn.ac.uk, dccp@vger.kernel.org, coreteam@netfilter.org, Jozsef Kadlecsik , Andrey Ryabinin , linux-ext4@vger.kernel.org, Alexey Kuznetsov , Pablo Neira Ayuso , linux-s390 , Andrey Konovalov , intel-gfx , Ursula Braun , Rodrigo Vivi , Dmitry Vyukov , Theodore Ts'o , Hideaki YOSHIFUJI , Greg Kroah-Hartman , Florian Westphal Linux Kernel Mailing List List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBKdWwgMzEsIDIwMTggYXQgMTA6NDkgQU0gTGludXMgVG9ydmFsZHMKPHRvcnZhbGRz QGxpbnV4LWZvdW5kYXRpb24ub3JnPiB3cm90ZToKPgo+IFNvIHRoZSByZS11c2UgbWlnaHQgaW5p dGlhbGl6ZSB0aGUgZmllbGRzIGxhemlseSwgbm90IG5lY2Vzc2FyaWx5IHVzaW5nIGEgY3Rvci4K CkluIHBhcnRpY3VsYXIsIHRoZSBwYXR0ZXJuIHRoYXQgbmZfY29ubnRyYWNrIHVzZXMgbG9va3Mg bGlrZSBpdCBpcyBzYWZlLgoKSWYgeW91IGhhdmUgYSB3ZWxsLWRlZmluZWQgcmVmY291bnQsIGFu ZCB1c2UgImF0b21pY19pbmNfbm90X3plcm8oKSIKdG8gZ3VhcmQgdGhlIHNwZWN1bGF0aXZlIFJD VSBhY2Nlc3Mgc2VjdGlvbiwgYW5kIHVzZQoiYXRvbWljX2RlY19hbmRfdGVzdCgpIiBpbiB0aGUg ZnJlZWluZyBzZWN0aW9uLCB0aGVuIHlvdSBzaG91bGQgYmUKc2FmZSB3cnQgbmV3IGFsbG9jYXRp b25zLgoKSWYgeW91IGhhdmUgYSBjb21wbGV0ZWx5IG5ldyBhbGxvY2F0aW9uIHRoYXQgaGFzICJy YW5kb20gc3RhbGUKY29udGVudCIsIHlvdSBrbm93IHRoYXQgaXQgY2Fubm90IGJlIG9uIHRoZSBS Q1UgbGlzdCwgc28gdGhlcmUgaXMgbm8Kc3BlY3VsYXRpdmUgYWNjZXNzIHRoYXQgY2FuIGV2ZXIg c2VlIHRoYXQgcmFuZG9tIGNvbnRlbnQuCgpTbyB0aGUgb25seSBjYXNlIHlvdSBuZWVkIHRvIHdv cnJ5IGFib3V0IGlzIGEgcmUtdXNlIGFsbG9jYXRpb24sIGFuZAp5b3Uga25vdyB0aGF0IHRoZSBy ZWZjb3VudCB3aWxsIHN0YXJ0IG91dCBhcyB6ZXJvIGV2ZW4gaWYgeW91IGRvbid0CmhhdmUgYSBj b25zdHJ1Y3Rvci4KClNvIHlvdSBjYW4gdGhpbmsgb2YgdGhlIHJlZmNvdW50IGl0c2VsZiBhcyBh bHdheXMgaGF2aW5nIGEgemVybwpjb25zdHJ1Y3RvciwgKkJVVCogeW91IG5lZWQgdG8gYmUgY2Fy ZWZ1bCB3aXRoIG9yZGVyaW5nLgoKSW4gcGFydGljdWxhciwgd2hvZXZlciBkb2VzIHRoZSBhbGxv Y2F0aW9uIG5lZWRzIHRvIHRoZW4gc2V0IHRoZQpyZWZjb3VudCB0byBhIG5vbi16ZXJvIHZhbHVl ICphZnRlciogaXQgaGFzIGluaXRpYWxpemVkIGFsbCB0aGUgb3RoZXIKZmllbGRzLiBBbmQgaW4g cGFydGljdWxhciwgaXQgbmVlZHMgdG8gbWFrZSBzdXJlIHRoYXQgaXQgdXNlcyB0aGUKcHJvcGVy IG1lbW9yeSBvcmRlcmluZyB0byBkbyBzby4KCkFuZCBpbiB0aGlzIGNhc2UsIHdlIGhhdmUKCiAg c3RhdGljIHN0cnVjdCBuZl9jb25uICoKICBfX25mX2Nvbm50cmFja19hbGxvYyhzdHJ1Y3QgbmV0 ICpuZXQsCiAgewogICAgICAgIC4uLgogICAgICAgIGF0b21pY19zZXQoJmN0LT5jdF9nZW5lcmFs LnVzZSwgMCk7Cgp3aGljaCBpcyBhIG5vLW9wIGZvciB0aGUgcmUtdXNlIGNhc2UgKHdoZXRoZXIg cmFjaW5nIG9yIG5vdCwgc2luY2UgYW55CiJpbmNfbm90X3plcm8iIHVzZXJzIHdvbid0IHRvdWNo IGl0KSwgYnV0IGluaXRpYWxpemVzIGl0IHRvIHplcm8gZm9yCnRoZSAiY29tcGxldGVseSBuZXcg b2JqZWN0IiBjYXNlLgoKQW5kIHRoZW4sIHRoZSB0aGluZyB0aGF0IGFjdHVhbGx5IGV4cG9zZXMg aXQgdG8gdGhlIHNwZWN1bGF0aXZlIHdhbGtlcnMgZG9lczoKCiAgaW50CiAgbmZfY29ubnRyYWNr X2hhc2hfY2hlY2tfaW5zZXJ0KHN0cnVjdCBuZl9jb25uICpjdCkKICB7CiAgICAgICAgLi4uCiAg ICAgICAgc21wX3dtYigpOwogICAgICAgIC8qIFRoZSBjYWxsZXIgaG9sZHMgYSByZWZlcmVuY2Ug dG8gdGhpcyBvYmplY3QgKi8KICAgICAgICBhdG9taWNfc2V0KCZjdC0+Y3RfZ2VuZXJhbC51c2Us IDIpOwoKd2hpY2ggbWVhbnMgdGhhdCBpdCBzdGF5cyBhcyB6ZXJvIHVudGlsIGV2ZXJ5dGhpbmcg aXMgYWN0dWFsbHkgc2V0IHVwLAphbmQgdGhlbiB0aGUgb3B0aW1pc3RpYyB3YWxrZXIgY2FuIHVz ZSB0aGUgb3RoZXIgZmllbGRzIChpbmNsdWRpbmcKc3BpbmxvY2tzIGV0YykgdG8gdmVyaWZ5IHRo YXQgaXQncyBhY3R1YWxseSB0aGUgcmlnaHQgdGhpbmcuIFRoZQpzbXBfd21iKCkgbWVhbnMgdGhh dCB0aGUgcHJldmlvdXMgaW5pdGlhbGl6YXRpb24gcmVhbGx5IHdpbGwgYmUKdmlzaWJsZSBiZWZv cmUgdGhlIG9iamVjdCBpcyB2aXNpYmxlLgoKU2lkZSBub3RlOiBvbiBzb21lIGFyY2hpdGVjdHVy ZXMgaXQgbWlnaHQgaGVscCB0byBtYWtlIHRoYXQgInNtcF93bWIKLT4gYXRvbWljX3NldCgpIiBz ZXF1ZW5jZSBiZSBhbSAic21wX3N0b3JlX3JlbGVhc2UoKSIgaW5zdGVhZC4gRG9lc24ndAptYXR0 ZXIgb24geDg2LCBidXQgbWlnaHQgbWF0dGVyIG9uIGFybTY0LgoKTk9URSEgT25lIHRoaW5nIHRv IGJlIHZlcnkgd29ycmllZCBhYm91dCBpcyB0aGF0IHJlLWluaXRpYWxpemluZwp3aGF0ZXZlciBS Q1UgbGlzdHMgbWVhbnMgdGhhdCBub3cgdGhlIFJDVSB3YWxrZXIgbWF5IGJlIHdhbGtpbmcgb24g dGhlCndyb25nIGxpc3Qgc28gdGhlIHdhbGtlciBtYXkgZG8gdGhlIHJpZ2h0IHRoaW5nIGZvciB0 aGlzIHBhcnRpY3VsYXIKZW50cnksIGJ1dCBpdCBtYXkgbWlzcyB3YWxraW5nICpvdGhlciogZW50 cmllcy4gU28gdGhlbiB5b3UgY2FuIGdldApzcHVyaW91cyBsb29rdXAgZmFpbHVyZXMsIGJlY2F1 c2UgdGhlIFJDVSB3YWxrZXIgbmV2ZXIgd2Fsa2VkIGFsbCB0aGUKd2F5IHRvIHRoZSBlbmQgb2Yg dGhlIHJpZ2h0IGxpc3QuIFRoYXQgZW5kcyB1cCBiZWluZyBhIG11Y2ggbW9yZQpzdWJ0bGUgYnVn LgoKQnV0IHRoZSBuZl9jb25udHJhY2sgY2FzZSBzZWVtcyB0byBnZXQgdGhhdCByaWdodCB0b28s IHNlZSB0aGUgcmVzdGFydAppbiBfX19fbmZfY29ubnRyYWNrX2ZpbmQoKS4KClNvIEkgZG9uJ3Qg c2VlIGFueXRoaW5nIHdyb25nIGluIG5mX2Nvbm50cmFjay4KCkJ1dCB5ZXMsIHVzaW5nIFNMQUJf VFlQRVNBRkVfQllfUkNVIGlzIHZlcnkgdmVyeSBzdWJ0bGUuIEJ1dCBtb3N0IG9mCnRoZSBzdWJ0 bGV0aWVzIGhhdmUgbm90aGluZyB0byBkbyB3aXRoIGhhdmluZyBhIGNvbnN0cnVjdG9yLCB0aGV5 IGFyZQphYm91dCB0aG9zZSAibWFrZSBzdXJlIG1lbW9yeSBvcmRlcmluZyB3cnQgcmVmY291bnQg aXMgcmlnaHQiIGFuZAoicmVzdGFydCBzcGVjdWxhdGl2ZSBSQ1Ugd2FsayIgaXNzdWVzIHRoYXQg YWN0dWFsbHkgaGFwcGVuIHJlZ2FyZGxlc3MKb2YgaGF2aW5nIGEgY29uc3RydWN0b3Igb3Igbm90 LgoKICAgICAgICAgICAgICAgICAgTGludXMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Date: Tue, 31 Jul 2018 18:51:59 +0000 Subject: Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implement Message-Id: List-Id: References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: dccp@vger.kernel.org On Tue, Jul 31, 2018 at 10:49 AM Linus Torvalds wrote: > > So the re-use might initialize the fields lazily, not necessarily using a ctor. In particular, the pattern that nf_conntrack uses looks like it is safe. If you have a well-defined refcount, and use "atomic_inc_not_zero()" to guard the speculative RCU access section, and use "atomic_dec_and_test()" in the freeing section, then you should be safe wrt new allocations. If you have a completely new allocation that has "random stale content", you know that it cannot be on the RCU list, so there is no speculative access that can ever see that random content. So the only case you need to worry about is a re-use allocation, and you know that the refcount will start out as zero even if you don't have a constructor. So you can think of the refcount itself as always having a zero constructor, *BUT* you need to be careful with ordering. In particular, whoever does the allocation needs to then set the refcount to a non-zero value *after* it has initialized all the other fields. And in particular, it needs to make sure that it uses the proper memory ordering to do so. And in this case, we have static struct nf_conn * __nf_conntrack_alloc(struct net *net, { ... atomic_set(&ct->ct_general.use, 0); which is a no-op for the re-use case (whether racing or not, since any "inc_not_zero" users won't touch it), but initializes it to zero for the "completely new object" case. And then, the thing that actually exposes it to the speculative walkers does: int nf_conntrack_hash_check_insert(struct nf_conn *ct) { ... smp_wmb(); /* The caller holds a reference to this object */ atomic_set(&ct->ct_general.use, 2); which means that it stays as zero until everything is actually set up, and then the optimistic walker can use the other fields (including spinlocks etc) to verify that it's actually the right thing. The smp_wmb() means that the previous initialization really will be visible before the object is visible. Side note: on some architectures it might help to make that "smp_wmb -> atomic_set()" sequence be am "smp_store_release()" instead. Doesn't matter on x86, but might matter on arm64. NOTE! One thing to be very worried about is that re-initializing whatever RCU lists means that now the RCU walker may be walking on the wrong list so the walker may do the right thing for this particular entry, but it may miss walking *other* entries. So then you can get spurious lookup failures, because the RCU walker never walked all the way to the end of the right list. That ends up being a much more subtle bug. But the nf_conntrack case seems to get that right too, see the restart in ____nf_conntrack_find(). So I don't see anything wrong in nf_conntrack. But yes, using SLAB_TYPESAFE_BY_RCU is very very subtle. But most of the subtleties have nothing to do with having a constructor, they are about those "make sure memory ordering wrt refcount is right" and "restart speculative RCU walk" issues that actually happen regardless of having a constructor or not. Linus