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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL 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 1E3E1C4338F for ; Wed, 18 Aug 2021 13:40:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A465E610A7 for ; Wed, 18 Aug 2021 13:40:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A465E610A7 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 188C88D0001; Wed, 18 Aug 2021 09:40:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 15E8E6B0072; Wed, 18 Aug 2021 09:40:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 074358D0001; Wed, 18 Aug 2021 09:40:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0180.hostedemail.com [216.40.44.180]) by kanga.kvack.org (Postfix) with ESMTP id DE81C6B006C for ; Wed, 18 Aug 2021 09:40:17 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 7568B18255A0D for ; Wed, 18 Aug 2021 13:40:17 +0000 (UTC) X-FDA: 78488310474.39.D8C88F9 Received: from mail-ot1-f42.google.com (mail-ot1-f42.google.com [209.85.210.42]) by imf05.hostedemail.com (Postfix) with ESMTP id 324D050548FE for ; Wed, 18 Aug 2021 13:40:17 +0000 (UTC) Received: by mail-ot1-f42.google.com with SMTP id 108-20020a9d01750000b029050e5cc11ae3so3551715otu.5 for ; Wed, 18 Aug 2021 06:40:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=92kjzJHPbu8CdHkXkohUqea/s0XZxMJWcWOY/L8K0Q0=; b=eqlv99p38UbOHhb+iaqlx8R7pajxw88f0tDyRo/zUVE14LEO4nwE2X3XKvBkTLTGo9 fvZ1OU5V9As8yW7DKQiDEIcigagyaC8S/UD1sEutBruhxBh268wCIylG2YzXXgG3Z7AT bSKuRtM+aMBRCld1BcmNLRbNFDaH5uXdhOs2UyOOCAjv8KdUznngHKeegRULI8Dzwhgw 83/pzM0Hy3rY6gc9LoeNVduSmNCBEvcklG+qOxxJbJXVD0fp5sIDxOdfVTx0mvdN16A5 3M7ncvOWOapHbHzd6Cxc7trD4qzvbv6XgEZdYKhUgI+M9hg/mGxjGUC+YoFplWTrRaHs qvsA== 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=92kjzJHPbu8CdHkXkohUqea/s0XZxMJWcWOY/L8K0Q0=; b=qwcb3FT1rQ1aZ6kWiJE5wPrM9jUUDP0iDdSTwHFp/iapk8ES7/kBJFh3rDQ+A72DJV 4Mlqzx6qTIVx4MJv5AQEVKVDNr5SBakk5o13aXn3iv/SVSAI+Vea9ProexJEMl3Muzky VYFIptQ89rfURR+wPK6Qs8W/Z4AFvWe1BhYNnHGa+tspTjgK0Vo/M/n3aDlt5gVsQ4Ql SzIRJOERaqetrv/ZyGHEtCcReyxBIgglU3ZP1ExmmX2PLWPPCbR7pZ6NmUDF2KOXZmAA gqprUbddHuAsJzg1Fvg9JQ/QR5w+e6m57fanqs9mJN5Dbi9dt/583XFupFUdnilI7eFD yikg== X-Gm-Message-State: AOAM532yoFgORoufnhgs0IpKBiV7ODGfEIS+wXKwaIxPcai/NpsBAobw aus5Jmk9UHOCoklHskS8oN7nnzAB0OSk5mfjnFKejA== X-Google-Smtp-Source: ABdhPJypTijaCEpAT80SQ6v46jAeX/zU9T+cs9V7Lj3fV7JwJH8rlSesYnznhG565RDZRzgFMQJ7ljRZ5rCVraf86LQ= X-Received: by 2002:a05:6830:3114:: with SMTP id b20mr3849263ots.17.1629294016304; Wed, 18 Aug 2021 06:40:16 -0700 (PDT) MIME-Version: 1.0 References: <20210818130300.2482437-1-elver@google.com> In-Reply-To: <20210818130300.2482437-1-elver@google.com> From: Marco Elver Date: Wed, 18 Aug 2021 15:40:04 +0200 Message-ID: Subject: Re: [PATCH] kfence: fix is_kfence_address() for addresses below KFENCE_POOL_SIZE To: elver@google.com, akpm@linux-foundation.org Cc: glider@google.com, dvyukov@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kasan-dev@googlegroups.com, Kuan-Ying Lee , stable@vger.kernel.org, Jann Horn Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 324D050548FE Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=eqlv99p3; spf=pass (imf05.hostedemail.com: domain of elver@google.com designates 209.85.210.42 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam01 X-Stat-Signature: wa1koa7z3g1t9wetm97ddij8zjyiicx6 X-HE-Tag: 1629294017-568567 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: +Cc Jann On Wed, 18 Aug 2021 at 15:03, Marco Elver wrote: > > Originally the addr != NULL check was meant to take care of the case > where __kfence_pool == NULL (KFENCE is disabled). However, this does not > work for addresses where addr > 0 && addr < KFENCE_POOL_SIZE. > > This can be the case on NULL-deref where addr > 0 && addr < PAGE_SIZE or > any other faulting access with addr < KFENCE_POOL_SIZE. While the kernel > would likely crash, the stack traces and report might be confusing due > to double faults upon KFENCE's attempt to unprotect such an address. > > Fix it by just checking that __kfence_pool != NULL instead. > > Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure") > Reported-by: Kuan-Ying Lee > Signed-off-by: Marco Elver > Cc: [5.12+] > --- > include/linux/kfence.h | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/include/linux/kfence.h b/include/linux/kfence.h > index a70d1ea03532..3fe6dd8a18c1 100644 > --- a/include/linux/kfence.h > +++ b/include/linux/kfence.h > @@ -51,10 +51,11 @@ extern atomic_t kfence_allocation_gate; > static __always_inline bool is_kfence_address(const void *addr) > { > /* > - * The non-NULL check is required in case the __kfence_pool pointer was > - * never initialized; keep it in the slow-path after the range-check. > + * The __kfence_pool != NULL check is required to deal with the case > + * where __kfence_pool == NULL && addr < KFENCE_POOL_SIZE. Keep it in > + * the slow-path after the range-check! > */ > - return unlikely((unsigned long)((char *)addr - __kfence_pool) < KFENCE_POOL_SIZE && addr); > + return unlikely((unsigned long)((char *)addr - __kfence_pool) < KFENCE_POOL_SIZE && __kfence_pool); > } Jann, I recall discussing this check somewhere around: https://lore.kernel.org/linux-doc/CAG48ez0D1+hStZaDOigwbqNqFHJAJtXK+8Nadeuiu1Byv+xp5A@mail.gmail.com/ I think you pointed out initially that we need another check, but somehow that turned into '&& addr' -- I think that's what we ended up with because of worry about another memory load, which is clearly wrong as that only works if addr==NULL. Simply checking __kfence_pool!=NULL is enough. I also checked codegen, and the compiler is smart enough to not reload the global __kfence_pool. Wanted to call it out, just in case you see something even more efficient (probably the only way to do better is to get rid of the 2nd branch, which I don't think is possible). :-) Thanks, -- Marco