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 8C9D8C4338F for ; Wed, 18 Aug 2021 13:43:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 32CE76108F for ; Wed, 18 Aug 2021 13:43:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 32CE76108F 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 68C616B006C; Wed, 18 Aug 2021 09:43:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 63CC66B0072; Wed, 18 Aug 2021 09:43:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 52B7F8D0001; Wed, 18 Aug 2021 09:43:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0163.hostedemail.com [216.40.44.163]) by kanga.kvack.org (Postfix) with ESMTP id 38CCE6B006C for ; Wed, 18 Aug 2021 09:43:48 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E1729250DA for ; Wed, 18 Aug 2021 13:43:47 +0000 (UTC) X-FDA: 78488319294.27.8F8E999 Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) by imf29.hostedemail.com (Postfix) with ESMTP id 7DEB3901E0D4 for ; Wed, 18 Aug 2021 13:43:47 +0000 (UTC) Received: by mail-qk1-f179.google.com with SMTP id n11so3033994qkk.1 for ; Wed, 18 Aug 2021 06:43:47 -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:content-transfer-encoding; bh=S5eDEdgJSEI6ifHq/vU3nLhIVHOqJcFRYhjJl8EuhE0=; b=Pj9w/afK4xCzTgMwxRehQ4cL3ZQPYEgqHGelBemKLayuGCdC/u/UQ7GJN/pVLWLFLz 0wDpiapzrWdc/40d0vQF+Mbo6J2QYXmAIiqJq7X69VSww6XYpCy3xJGgY22JNCI5JcRx A+EoxuqAdKVG4w+4Gb58kJCvte0r2AazrRVOG/R2dxBd33f1KsLZSJobusbiI3IJSFrS NwU3wmxwAae8Smx9NmCXgRWXQ68c87R/8XhB61knpl/zCj8GGV3A5/w+GhMtApXAfeQ4 lqqkwVCkE4l6QYdWE7tvHbhu4bvpdFGpB9Shu0s1w9Q9+Q8Ure+NROOQVPlG7mSsbnb7 kI7Q== 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:content-transfer-encoding; bh=S5eDEdgJSEI6ifHq/vU3nLhIVHOqJcFRYhjJl8EuhE0=; b=RlPNKp+ReXk4WqF0S6C4h2Z0bzGe7Z/pmlXGfSMPNLJ3ot2Veij2UUMON7Rm7Tr6k2 QBWjTPhOb2NoNqcQrk/XfpttfXZqqbqDHvUkPQBfnoOTL7Wvvvu0ZI3DtZjyhY69e83W e+z4HYyYS6ZEoOU1oNJlCYS3NThDseDlrMd+KLEJm7fRYiw2J+6c6UTNa1xXaZqG7eX/ 68s0iNEK9qlRIM0j1WFVl1NzgEsKDpmsfPzLpqfxY1oova/fCVJTdo0KM6A9HGpHd+hm /Bkj9PLq+xaDlmAx2C8cpEifb+KW9IdxF3wuQE+2lnk1D2n24BROtl5dwA6I3U0p930W Bcdw== X-Gm-Message-State: AOAM5317l3KPR6RAP4zn5kxnrqgnYRY7j4Tr4I4h7ztHnrhafNn9IbJh n1bx0ud2XkL93QCH7mAFf8t4dpl9K5cjk1+WBAIAcQ== X-Google-Smtp-Source: ABdhPJzl8uL1pta8qcUndh6lp7PpcHPpAa7sjxUZA4U7T7YY7r2wNM73Pq55xSwo7n9r1hQDFzSnZCqDCNWgbSsG/YA= X-Received: by 2002:a05:620a:d54:: with SMTP id o20mr9700090qkl.326.1629294226642; Wed, 18 Aug 2021 06:43:46 -0700 (PDT) MIME-Version: 1.0 References: <20210818130300.2482437-1-elver@google.com> In-Reply-To: From: Alexander Potapenko Date: Wed, 18 Aug 2021 15:43:09 +0200 Message-ID: Subject: Re: [PATCH] kfence: fix is_kfence_address() for addresses below KFENCE_POOL_SIZE To: Marco Elver Cc: Andrew Morton , Dmitriy Vyukov , LKML , Linux Memory Management List , kasan-dev , Kuan-Ying Lee , stable@vger.kernel.org, Jann Horn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b="Pj9w/afK"; spf=pass (imf29.hostedemail.com: domain of glider@google.com designates 209.85.222.179 as permitted sender) smtp.mailfrom=glider@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 7DEB3901E0D4 X-Stat-Signature: k8o7j38y4p1rho3cpsu7qf5e7wgiyj8f X-HE-Tag: 1629294227-181742 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 Wed, Aug 18, 2021 at 3:40 PM Marco Elver wrote: > > +Cc Jann > > On Wed, 18 Aug 2021 at 15:03, Marco Elver wrote: > > > > Originally the addr !=3D NULL check was meant to take care of the case > > where __kfence_pool =3D=3D NULL (KFENCE is disabled). However, this doe= s 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 o= r > > any other faulting access with addr < KFENCE_POOL_SIZE. While the kerne= l > > 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 !=3D NULL instead. > > > > Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure") > > Reported-by: Kuan-Ying Lee > > Signed-off-by: Marco Elver Acked-by: Alexander Potapenko > > 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 poi= nter was > > - * never initialized; keep it in the slow-path after the range-= check. > > + * The __kfence_pool !=3D NULL check is required to deal with t= he case > > + * where __kfence_pool =3D=3D 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+8Nade= uiu1Byv+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=3D=3DNULL. Simply checking > __kfence_pool!=3DNULL 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 --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg