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=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable 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 99E70C4741F for ; Thu, 5 Nov 2020 11:11:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D66CA2224B for ; Thu, 5 Nov 2020 11:11:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="UgAG4I4R" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D66CA2224B Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 240F86B00DF; Thu, 5 Nov 2020 06:11:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1A3646B00E0; Thu, 5 Nov 2020 06:11:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 068B46B00E1; Thu, 5 Nov 2020 06:11:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0238.hostedemail.com [216.40.44.238]) by kanga.kvack.org (Postfix) with ESMTP id C58306B00DF for ; Thu, 5 Nov 2020 06:11:32 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 69EFB8249980 for ; Thu, 5 Nov 2020 11:11:32 +0000 (UTC) X-FDA: 77450098824.23.shade27_5412c6e272c9 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id 495BE37606 for ; Thu, 5 Nov 2020 11:11:32 +0000 (UTC) X-HE-Tag: shade27_5412c6e272c9 X-Filterd-Recvd-Size: 6234 Received: from mail-oi1-f196.google.com (mail-oi1-f196.google.com [209.85.167.196]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Thu, 5 Nov 2020 11:11:31 +0000 (UTC) Received: by mail-oi1-f196.google.com with SMTP id k26so1294440oiw.0 for ; Thu, 05 Nov 2020 03:11:31 -0800 (PST) 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=p0c7rtJJXojaI2Rh6wYkHrsZ57LiUlQdb8npxhv/hGc=; b=UgAG4I4ReO+wn1wDSqRbyoCy/IJUjdK11POtyqZqUfpFft8GE5GL+OCgDZNIMC2ShI Ak0T5FghDwJA9fsgRUMCNqvCZ5ROdofuxkSrFoWhSKgehoYNxd8xAA0kUt+88ZgFMdBX xgsxR0H5vGTRnPChZLgASs3o9ItRAZ6/W6Y7TEIlhnSDqu8jPyq7LyRZbke4U7dM6iLN FxDmcUCC8Kcmtvm2sajgAxr+hcGGJFTTEjTiXHHgQS9xy5QRr9+mozw1suvBw1OUxCTE m5zOLb1SNNObTwE/7RoI99npnj5OJ7hGTDK75JI2+K70tGiaVWVeWGxAC90Z4pKny19h p+Xg== 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=p0c7rtJJXojaI2Rh6wYkHrsZ57LiUlQdb8npxhv/hGc=; b=emp2ZaDSNJt6RrJCfwTO56sn7WigtLEuC1dqhVqjVb75cXramIMxnATFtn1Z2zkzYE +f5jttNHo74tm1g4jn/bN3H5/WmHc3pgVvJNTyoiu0zESJ6EjsssBDntQYAuYopSyeAL 9xweaxCaRYOlxdL2N/pb7iNy8MOXliVCOi360Ka18yBl/Ym5DivuqoDNsL49K9GyRp3a W8owqbuS81Pv/MRe5+7NZVmZ0rUjj4Tlpm4V+rRh7Ub/XlKVwaKKrwvil+YITP5PuvOW xSGIKR3SlEPsUjzMVL0gL8CNYcOOTzzbJC37UrV5ZCfaXcr+fVrFbUjsE9/pz1EOw/L0 tyqQ== X-Gm-Message-State: AOAM533tKYt7yCTbzaFIEe9ldrtrTaGY638zkMLi1vNFRg+Jq0xfpwvA D9/frblkIjIlKt32Wnuek/SArYhL9zjrU4bWMIRFag== X-Google-Smtp-Source: ABdhPJw/bKjx3NB4HnHeMfp7dUNwz337/g01wK6iBNiTZauyMStIy5uzOAY0RMwaytsPEAZJM2mVackSG5B6M6pOREs= X-Received: by 2002:aca:a988:: with SMTP id s130mr1214512oie.172.1604574690901; Thu, 05 Nov 2020 03:11:30 -0800 (PST) MIME-Version: 1.0 References: <20201105092133.2075331-1-elver@google.com> <20201105105241.GC82102@C02TD0UTHF1T.local> In-Reply-To: <20201105105241.GC82102@C02TD0UTHF1T.local> From: Marco Elver Date: Thu, 5 Nov 2020 12:11:19 +0100 Message-ID: Subject: Re: [PATCH] kfence: Use pt_regs to generate stack trace on faults To: Mark Rutland Cc: Andrew Morton , Alexander Potapenko , Dmitry Vyukov , Jann Horn , LKML , Linux Memory Management List , kasan-dev , "the arch/x86 maintainers" , Linux ARM , "Paul E. McKenney" Content-Type: text/plain; charset="UTF-8" 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 Thu, 5 Nov 2020 at 11:52, Mark Rutland wrote: > On Thu, Nov 05, 2020 at 10:21:33AM +0100, Marco Elver wrote: > > Instead of removing the fault handling portion of the stack trace based > > on the fault handler's name, just use struct pt_regs directly. > > > > Change kfence_handle_page_fault() to take a struct pt_regs, and plumb it > > through to kfence_report_error() for out-of-bounds, use-after-free, or > > invalid access errors, where pt_regs is used to generate the stack > > trace. > > > > If the kernel is a DEBUG_KERNEL, also show registers for more > > information. > > > > Suggested-by: Mark Rutland > > Signed-off-by: Marco Elver > > Wow; I wasn't expecting this to be put together so quickly, thanks for > doing this! > > From a scan, this looks good to me -- just one question below. > > > diff --git a/include/linux/kfence.h b/include/linux/kfence.h > > index ed2d48acdafe..98a97f9d43cd 100644 > > --- a/include/linux/kfence.h > > +++ b/include/linux/kfence.h > > @@ -171,6 +171,7 @@ static __always_inline __must_check bool kfence_free(void *addr) > > /** > > * kfence_handle_page_fault() - perform page fault handling for KFENCE pages > > * @addr: faulting address > > + * @regs: current struct pt_regs (can be NULL, but shows full stack trace) > > * > > * Return: > > * * false - address outside KFENCE pool, > > > @@ -44,8 +44,12 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries > > case KFENCE_ERROR_UAF: > > case KFENCE_ERROR_OOB: > > case KFENCE_ERROR_INVALID: > > - is_access_fault = true; > > - break; > > + /* > > + * kfence_handle_page_fault() may be called with pt_regs > > + * set to NULL; in that case we'll simply show the full > > + * stack trace. > > + */ > > + return 0; > > For both the above comments, when/where is kfence_handle_page_fault() > called with regs set to NULL? I couldn't spot that in this patch, so > unless I mised it I'm guessing that's somewhere outside of the patch > context? Right, currently it's not expected to happen, but I'd like to permit this function being called not from fault handlers, for use-cases like this: https://lkml.kernel.org/r/CANpmjNNxAvembOetv15FfZ=04mpj0Qwx+1tnn22tABaHHRRv=Q@mail.gmail.com The revised recommendation when trying to get KFENCE to give us more information about allocation/free stacks after refcount underflow (like what Paul was trying to do) would be to call kfence_handle_page_fault(addr, NULL). > If this is a case we don't expect to happen, maybe add a WARN_ON_ONCE()? While it's currently not expected, I don't see why we should make this WARN and limit the potential uses of the API if it works just fine if we pass regs set to NULL. Although arguably the name kfence_handle_page_fault() might be confusing for such uses, for now, until more widespread use is evident (if at all) I'd say let's keep as-is, but simply not prevent such use-cases. Thanks, -- Marco