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 017F5C433F5 for ; Wed, 27 Apr 2022 08:45:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2D4936B0071; Wed, 27 Apr 2022 04:45:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 25C006B0072; Wed, 27 Apr 2022 04:45:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FEF16B0073; Wed, 27 Apr 2022 04:45:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id F14766B0071 for ; Wed, 27 Apr 2022 04:45:33 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id B544D61F8D for ; Wed, 27 Apr 2022 08:45:33 +0000 (UTC) X-FDA: 79402025346.03.B28E6B3 Received: from mail-qt1-f182.google.com (mail-qt1-f182.google.com [209.85.160.182]) by imf28.hostedemail.com (Postfix) with ESMTP id A51F2C004C for ; Wed, 27 Apr 2022 08:45:25 +0000 (UTC) Received: by mail-qt1-f182.google.com with SMTP id fu47so655040qtb.5 for ; Wed, 27 Apr 2022 01:45:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:from:to:cc:subject:references:mime-version :content-disposition:in-reply-to; bh=RB8+FqJ1IEgAxvX3bqMX+LwZyBF33/dYNV+lewufcOs=; b=YiudLaujLbqxUmAQ5oA0hXxrRSlSYMybJoHK7kRC6iIpCYEK7XdRtfbGQ2uq5GYXi3 ac4JZpTKvvpYLB8W+8iWO5quKkZhSu3u/4OkQIN8PxrjhtZJXbfg3oT0Vp5x+Cz64xzi lZFmazS4izkXhcG29TTT12ka2ErmX2c3ZOKog38Gn6UXcQnAhDyVeoRpl5C1ojTkH42/ mFbNO2RuksUCNwQALhwtf/kag7KxMMajgT9frnMz/4MwhPVRXoB33ndBNGVT8bv9Gu3y zjINfNt7FemtkmYJdXysv82NTRN/XF/WT/6QOlv7kJ8KjuV8uvf42RlImhQ/gGyR1XS7 JkJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:from:to:cc:subject:references :mime-version:content-disposition:in-reply-to; bh=RB8+FqJ1IEgAxvX3bqMX+LwZyBF33/dYNV+lewufcOs=; b=w+QosmRv9Wf0yEZoRHdyBnZ2ltsEU1MzipjyKL/gkZSi53cC5F7Qp9fzte3SO7NcIc euYddWii9YdoIjattgd6P8zDuPtpflFzvYWIEMBRJBOInDib+tLAmKnI65+r/ms8h8lr AG9CMv0/ER4mqZlXsC24z4Ff7R4XkSLgQdDmNY2vuTLiomPH6ZXIPawjLj3pS0J1ICqw YX1kyQN+d+ku4geHht7KnKDzNdwYm41e2asSbtpvL6iFZFFZoxrkefVoF7gtXwlaN4OY Tcjlcr6nhaWlBs6vmbC0+ZWcC0ryUOQ04rLtuqrnVPOeK/+OHvtFlpNQeXDyASyU6uq5 Ovsw== X-Gm-Message-State: AOAM531QNhmrubDBZAJA5U6Pvq4a4K/PZf0YTV2fU3pUcxmlB8YNN/Eo mv3/WEVR9wbg9wB4ruITHAA= X-Google-Smtp-Source: ABdhPJyocxPC0ZydQKbFq36fs4NYuBw9VXrJ5Oi4ajVGkBxamLejWLMn3aRuq6VJly/vNpkqn2nsgw== X-Received: by 2002:a05:622a:284:b0:2f2:bf5f:4bd9 with SMTP id z4-20020a05622a028400b002f2bf5f4bd9mr18174425qtw.503.1651049132446; Wed, 27 Apr 2022 01:45:32 -0700 (PDT) Received: from localhost ([193.203.214.57]) by smtp.gmail.com with ESMTPSA id p12-20020a05622a00cc00b002ebdd6ef303sm10061443qtw.43.2022.04.27.01.45.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Apr 2022 01:45:31 -0700 (PDT) Message-ID: <626902ab.1c69fb81.44f01.9c06@mx.google.com> X-Google-Original-Message-ID: <20220427084529.GA3844528@cgel.zte@gmail.com> Date: Wed, 27 Apr 2022 08:45:29 +0000 From: CGEL To: Marco Elver Cc: glider@google.com, akpm@linux-foundation.org, dvyukov@google.com, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, xu xin , Zeal Robot Subject: Re: [PATCH] mm/kfence: fix a potential NULL pointer dereference References: <20220427071100.3844081-1-xu.xin16@zte.com.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: A51F2C004C X-Stat-Signature: webxgmfyumpppmt9xhoh35d9ed64iw79 X-Rspam-User: Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=YiudLauj; spf=pass (imf28.hostedemail.com: domain of cgel.zte@gmail.com designates 209.85.160.182 as permitted sender) smtp.mailfrom=cgel.zte@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1651049125-821617 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, Apr 27, 2022 at 09:33:52AM +0200, Marco Elver wrote: > On Wed, 27 Apr 2022 at 09:11, wrote: > > > > From: xu xin > > > > In __kfence_free(), the returned 'meta' from addr_to_metadata() > > might be NULL just as the implementation of addr_to_metadata() > > shows. > > > > Let's add a check of the pointer 'meta' to avoid NULL pointer > > dereference. The patch brings three changes: > > > > 1. Add checks in both kfence_free() and __kfence_free(); > > 2. kfence_free is not inline function any longer and new inline > > function '__try_free_kfence_meta' is introduced. > > This is very bad for performance (see below). > > > 3. The check of is_kfence_address() is not required for > > __kfence_free() now because __kfence_free has done the check in > > addr_to_metadata(); > > > > Reported-by: Zeal Robot > > Is this a static analysis robot? Please show a real stack trace with > an actual NULL-deref. > > Nack - please see: > https://lore.kernel.org/all/CANpmjNO5-o1B9r2eYS_482RBVJSyPoHSnV2t+M8fJdFzBf6d2A@mail.gmail.com/ > Thanks for your reply. It's from static analysis indeed and no actual NULL-deref event happened yet. I'm just worried that what if address at the edge of __kfence_pool and thus addr_to_metadata() returns NULL. Is is just a guess, I'm not sure. But if __kfence_free make sure that the given address never is at the edge of __kfence_pool, then the calculation and check in addr_to_metadata() is extra performance consumption: "index = (addr - (unsigned long)__kfence_pool) / (PAGE_SIZE * 2) - 1; if (index < 0 || index >= CONFIG_KFENCE_NUM_OBJECTS) 240 return NULL;" > >Signed-off-by: xu xin > > --- > > include/linux/kfence.h | 10 ++-------- > > mm/kfence/core.c | 30 +++++++++++++++++++++++++++--- > > 2 files changed, 29 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/kfence.h b/include/linux/kfence.h > > index 726857a4b680..fbf6391ab53c 100644 > > --- a/include/linux/kfence.h > > +++ b/include/linux/kfence.h > > @@ -160,7 +160,7 @@ void *kfence_object_start(const void *addr); > > * __kfence_free() - release a KFENCE heap object to KFENCE pool > > * @addr: object to be freed > > * > > - * Requires: is_kfence_address(addr) > > + * Requires: is_kfence_address(addr), but now it's unnecessary > > (As an aside, something can't be required and be unnecessary at the same time.) Oh, I'm sorry for this. In my opinion, inner addr_to_metadata(), is_kfence_address is executed for the second time, so not necessary here.