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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 87453C636D7 for ; Thu, 23 Feb 2023 04:08:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=0yS31k1bHFphUTMfUtJQHOb23mGXvlfTQO1V9F17mqg=; b=N3B/Imez1ZU6VM /uQCpoX6XPbPs9pJEwdL4jBZYEtJK3jlfiNSCFstMTUJG0ckyGeKTz1zA3aJBR1JY34XNThRTvVdH i11yDnoOAvuXvJG+GZM2fni5h08o/GfEUlPa6o/B0LNlsMVNsgrAJ5kEFdUvkuawzmRlilKqH2W5o 5ogU+RYC70X1/RKDqseHotn67gGEr/k6DhtcoPFRQPu0SkrpdkXWNnpEl0fjOdbA8Bm5RWF09wG+3 EMnX8aa9PaJGWaIcRCN57phkbwCXkDJ0DSpNuSYvCS9dr/L2VBTsDcFzOE9mQdzBt5NmwrNVRJCBv /2WucbbVdmWCU9YW6H5w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pV2tJ-00Evd2-FG; Thu, 23 Feb 2023 04:07:33 +0000 Received: from mail-pj1-x102d.google.com ([2607:f8b0:4864:20::102d]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pV2tG-00EvcC-GG for linux-arm-kernel@lists.infradead.org; Thu, 23 Feb 2023 04:07:31 +0000 Received: by mail-pj1-x102d.google.com with SMTP id cp7-20020a17090afb8700b0023756229427so2458130pjb.1 for ; Wed, 22 Feb 2023 20:07:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to:date :cc:to:from:subject:message-id:from:to:cc:subject:date:message-id :reply-to; bh=oWSEd1NxYC11TmhqLYnTRyNanS7IntnXRdA2JcgjIq4=; b=A6xKQmhsb0+t3BEJkyFlkIOkRBQwx77Yv4++QoGR7PUIzHXLjhkzOpopYLPVx2BbWb NrSAfaSlzVUD4fy7wK2vqi2cqNG1hkR4qFHoPL2IyNJx6yXu1Cy5anJhi901uCzcICRQ QAYsMD9+qVsQfbz9+xGLl665DurwWY9iITEFqxZRwSptRfZZtrMTNvWYE0RmFmMNASgY cj9QBxrskW20mNqSplSd5GCQTNEMRXSO/n7Rv0O2s1WhlAnV42j/mRUf/sO0ulhQpZwQ 3SIaY/osDqNjJfx9llwn3UoRBGclW7PwsZW0ejABRIu7YQh0xtbrlAF9dOnure6b6LoA hxPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to:date :cc:to:from:subject:message-id:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=oWSEd1NxYC11TmhqLYnTRyNanS7IntnXRdA2JcgjIq4=; b=3WzWGQPRUCoj50ia9BnMDmOk/aLexfFfbMubhXW7BdqdPFoPnpGKvgeUyxhro1DWek MV4n/WSOG4oj9TgPDuUcrdJ4WyTtBdWpAitUAfsupamFieatmVrJnl6cl2TxAeo451hm zDJb+8SN715oibhr/a1V+Ali8yXlNlvscQWdlf/+7bXBraMFCOekaEzdWJtti7qKzFzW uR2my5+NgKSxKvrq2dVmGxymGIVWVS4Yf2/tMWRPQjvkxlcZt29QAeJcRGrd4vykth0R 39ZuC2KipCywP13STnt5QUoRy3RUf0PkSibURmKEesiLoYJpxy4qDlsAx6trz3x8dyOg BYzw== X-Gm-Message-State: AO0yUKUoS2kW+IrM0JkQ4QbeZNwfh8VXixgrn629dSwKlJ3MgkzwqnBO Dtjf9+I8VWkaclRT5OQpV78= X-Google-Smtp-Source: AK7set+JvYa165vssA3FxAgXlIB6DbNGxRw2hl3MGkRtnutUbOSThX5fn9dhRPgrTQR7/O7qY8zBIQ== X-Received: by 2002:a17:90b:4a4d:b0:234:148:4b27 with SMTP id lb13-20020a17090b4a4d00b0023401484b27mr13371708pjb.17.1677125247646; Wed, 22 Feb 2023 20:07:27 -0800 (PST) Received: from cdg1-dhcp-7-226.amazon.fr ([54.240.193.0]) by smtp.googlemail.com with ESMTPSA id gi4-20020a17090b110400b00230ab56a1f3sm5802814pjb.51.2023.02.22.20.07.21 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 22 Feb 2023 20:07:26 -0800 (PST) Message-ID: <88ab8c8348373e5c7c90c985dd92b5e06f32b16b.camel@gmail.com> Subject: Re: [RFC PATCH v3 19/22] arm64: unwinder: Add a reliability check in the unwinder based on ORC From: Suraj Jitindar Singh To: madvenka@linux.microsoft.com Cc: poimboe@redhat.com, peterz@infradead.org, chenzhongjin@huawei.com, mark.rutland@arm.com, broonie@kernel.org, nobuta.keiya@fujitsu.com, catalin.marinas@arm.com, will@kernel.org, jamorris@linux.microsoft.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Date: Thu, 23 Feb 2023 14:07:19 +1000 In-Reply-To: <20230202074036.507249-20-madvenka@linux.microsoft.com> References: <0337266cf19f4c98388e3f6d09f590d9de258dc7> <20230202074036.507249-1-madvenka@linux.microsoft.com> <20230202074036.507249-20-madvenka@linux.microsoft.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230222_200730_597828_0AA6602F X-CRM114-Status: GOOD ( 24.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 2023-02-02 at 01:40 -0600, madvenka@linux.microsoft.com wrote: > From: "Madhavan T. Venkataraman" > > Introduce a reliability flag in struct unwind_state. This will be set > to > false if the PC does not have a valid ORC or if the frame pointer > computed > from the ORC does not match the actual frame pointer. > > Now that the unwinder can validate the frame pointer, introduce > arch_stack_walk_reliable(). > > Signed-off-by: Madhavan T. Venkataraman > > --- > arch/arm64/include/asm/stacktrace/common.h | 15 ++ > arch/arm64/kernel/stacktrace.c | 167 > ++++++++++++++++++++- > 2 files changed, 175 insertions(+), 7 deletions(-) > [snip] > -static void notrace unwind(struct unwind_state *state, > +static int notrace unwind(struct unwind_state *state, bool > need_reliable, > stack_trace_consume_fn consume_entry, void > *cookie) > { > - while (1) { > - int ret; > + int ret = 0; > > + while (1) { > + if (need_reliable && !state->reliable) > + return -EINVAL; > if (!consume_entry(cookie, state->pc)) > break; > ret = unwind_next(state); > + if (need_reliable && !ret) > + unwind_check_reliable(state); > if (ret < 0) > break; > } > + return ret; nit: I think you're looking more for comments on the approach and the correctness of these patches, but from an initial read I'm still putting it all together in my head. So this comment is on the coding style. The above loop seems to check the current reliability state, then unwind a frame then check the reliability, and then break based of something which couldn't have been updated by the line immediately above. I propose something like: unwind(...) { ret = 0; while (!ret) { if (need_reliable) { unwind_check_reliable(state); if (!state->reliable) return -EINVAL; } if (!consume_entry(cookie, state->pc)) return -EINVAL; ret = unwind_next(state); } return ret; } This also removes the need for the call to unwind_check_reliable() before the first unwind() below in arch_stack_walk_reliable(). - Suraj > } > NOKPROBE_SYMBOL(unwind); > > @@ -216,5 +337,37 @@ noinline notrace void > arch_stack_walk(stack_trace_consume_fn consume_entry, > unwind_init_from_task(&state, task); > } > > - unwind(&state, consume_entry, cookie); > + unwind(&state, false, consume_entry, cookie); > +} > + > +noinline notrace int arch_stack_walk_reliable( > + stack_trace_consume_fn consume_entry, > + void *cookie, struct task_struct *task) > +{ > + struct stack_info stacks[] = { > + stackinfo_get_task(task), > + STACKINFO_CPU(irq), > +#if defined(CONFIG_VMAP_STACK) > + STACKINFO_CPU(overflow), > +#endif > +#if defined(CONFIG_VMAP_STACK) && defined(CONFIG_ARM_SDE_INTERFACE) > + STACKINFO_SDEI(normal), > + STACKINFO_SDEI(critical), > +#endif > + }; > + struct unwind_state state = { > + .stacks = stacks, > + .nr_stacks = ARRAY_SIZE(stacks), > + }; > + int ret; > + > + if (task == current) > + unwind_init_from_caller(&state); > + else > + unwind_init_from_task(&state, task); > + unwind_check_reliable(&state); > + > + ret = unwind(&state, true, consume_entry, cookie); > + > + return ret == -ENOENT ? 0 : -EINVAL; > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel