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=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, URIBL_BLOCKED,USER_AGENT_SANE_2 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 A91E4C433E3 for ; Mon, 20 Jul 2020 08:23:13 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7AF1220709 for ; Mon, 20 Jul 2020 08:23:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="MDUeZdtW"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="EmRoRdzv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7AF1220709 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Date: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=O+hiZI+MQGD50pW97llebhlc5f4JCVEgY9pftv6mQck=; b=MDUeZdtWyRcYCdanJhhjCrWik gxw0xQLKJ0kNjl3Qj5uPVggMrapxXt3+qfZzsDnev9acUWUa9p9O7GxUbv+M829dTlg2owB74L+bC pTulk9uR28ena8rn62YIEKcMHHFgySfWHYLscWAN+0IXj92/CzvVIPpss9HjxZO74L+NWBX6bWAtL O7+V/0ch5I9MdQDzMLk3xPoTBiVf8Ab6LsROdTU9ED3+rGhSrrA1byzXogYbEaaJaZ6gpdTGo1OD6 ep5AvyiltF7pKxEDjLr+xGkDwWg13DjKgKtDAcJ69LHd3G++iuZI9GZrEcLZd9U6SLU/dk3k/DWUL u7LP2937A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jxR3Z-000444-SC; Mon, 20 Jul 2020 08:21:53 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jxR3V-00042d-Hv; Mon, 20 Jul 2020 08:21:50 +0000 X-UUID: e08dad963e614a7f8897b80585bf8fac-20200720 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=g1ZpCrFRRkITcy2/cYkX9ggNIKxoHUd6Hg4M3iYk/hs=; b=EmRoRdzvyiJ7YfDfg0xck/kqjw5Cho7iDcNwuZQGo1m0eYKbDVACFpkkAIn20GBwcsoQmV72HdwDiRPFrjSVhK8j4rpejq2ICS8Tx9Dkn9eICK6DOLscXwLM58h0cQIIWgJoVp3bEPbiEKe/WI3aKRrnszzU0WNxQeKlmsE31Zo=; X-UUID: e08dad963e614a7f8897b80585bf8fac-20200720 Received: from mtkcas66.mediatek.inc [(172.29.193.44)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 1578997898; Mon, 20 Jul 2020 00:22:13 -0800 Received: from MTKMBS01N1.mediatek.inc (172.21.101.68) by MTKMBS62N1.mediatek.inc (172.29.193.41) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 20 Jul 2020 01:21:38 -0700 Received: from MTKCAS06.mediatek.inc (172.21.101.30) by mtkmbs01n1.mediatek.inc (172.21.101.68) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 20 Jul 2020 16:21:32 +0800 Received: from [172.21.77.33] (172.21.77.33) by MTKCAS06.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 20 Jul 2020 16:21:33 +0800 Message-ID: <1595233294.8055.0.camel@mtkswgap22> Subject: Re: [PATCH v2] cpuidle: change enter_s2idle() prototype From: Neal Liu To: "Rafael J. Wysocki" Date: Mon, 20 Jul 2020 16:21:34 +0800 In-Reply-To: <1594350535.4670.13.camel@mtkswgap22> References: <1594005196-16327-1-git-send-email-neal.liu@mediatek.com> <1594005196-16327-2-git-send-email-neal.liu@mediatek.com> <1594350535.4670.13.camel@mtkswgap22> X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200720_042149_751544_690955D0 X-CRM114-Status: GOOD ( 34.55 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jacob Pan , linux-tegra , wsd_upstream , Linux PM , Daniel Lezcano , "Rafael J. Wysocki" , lkml , Jonathan Hunter , ACPI Devel Maling List , Thierry Reding , Neal Liu , Sami Tolvanen , Matthias Brugger , "moderated list:ARM/Mediatek SoC..." , Linux ARM , Len Brown 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 Gentle ping on this patch. On Fri, 2020-07-10 at 11:08 +0800, Neal Liu wrote: > On Thu, 2020-07-09 at 14:18 +0200, Rafael J. Wysocki wrote: > > On Mon, Jul 6, 2020 at 5:13 AM Neal Liu wrote: > > > > > > Control Flow Integrity(CFI) is a security mechanism that disallows > > > changes to the original control flow graph of a compiled binary, > > > making it significantly harder to perform such attacks. > > > > > > init_state_node() assign same function callback to different > > > function pointer declarations. > > > > > > static int init_state_node(struct cpuidle_state *idle_state, > > > const struct of_device_id *matches, > > > struct device_node *state_node) { ... > > > idle_state->enter = match_id->data; ... > > > idle_state->enter_s2idle = match_id->data; } > > > > > > Function declarations: > > > > > > struct cpuidle_state { ... > > > int (*enter) (struct cpuidle_device *dev, > > > struct cpuidle_driver *drv, > > > int index); > > > > > > void (*enter_s2idle) (struct cpuidle_device *dev, > > > struct cpuidle_driver *drv, > > > int index); }; > > > > > > In this case, either enter() or enter_s2idle() would cause CFI check > > > failed since they use same callee. > > > > Can you please explain this in a bit more detail? > > > > As it stands, I don't understand the problem statement enough to apply > > the patch. > > > > Okay, Let's me try to explain more details. > Control Flow Integrity(CFI) is a security mechanism that disallows > changes to the original control flow graph of a compiled binary, making > it significantly harder to perform such attacks. > > There are multiple control flow instructions that could be manipulated > by the attacker and subvert control flow. The target instructions that > use data to determine the actual destination. > - indirect jump > - indirect call > - return > > In this case, function prototype between caller and callee are mismatch. > Caller: (type A)funcA > Callee: (type A)funcB > Callee: (type C)funcC > > funcA calls funcB -> no problem > funcA calls funcC -> CFI check failed > > That's why we try to align function prototype. > Please feel free to feedback if you have any questions. > > > > Align function prototype of enter() since it needs return value for > > > some use cases. The return value of enter_s2idle() is no > > > need currently. > > > > So last time I requested you to document why ->enter_s2idle needs to > > return an int in the code, which has not been done. Please do that. > > > > > Signed-off-by: Neal Liu > > > --- > > > drivers/acpi/processor_idle.c | 6 ++++-- > > > drivers/cpuidle/cpuidle-tegra.c | 8 +++++--- > > > drivers/idle/intel_idle.c | 6 ++++-- > > > include/linux/cpuidle.h | 6 +++--- > > > 4 files changed, 16 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > > index 75534c5..6ffb6c9 100644 > > > --- a/drivers/acpi/processor_idle.c > > > +++ b/drivers/acpi/processor_idle.c > > > @@ -655,8 +655,8 @@ static int acpi_idle_enter(struct cpuidle_device *dev, > > > return index; > > > } > > > > > > -static void acpi_idle_enter_s2idle(struct cpuidle_device *dev, > > > - struct cpuidle_driver *drv, int index) > > > +static int acpi_idle_enter_s2idle(struct cpuidle_device *dev, > > > + struct cpuidle_driver *drv, int index) > > > { > > > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > > > > > @@ -674,6 +674,8 @@ static void acpi_idle_enter_s2idle(struct cpuidle_device *dev, > > > } > > > } > > > acpi_idle_do_entry(cx); > > > + > > > + return 0; > > > } > > > > > > static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, > > > diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c > > > index 1500458..a12fb14 100644 > > > --- a/drivers/cpuidle/cpuidle-tegra.c > > > +++ b/drivers/cpuidle/cpuidle-tegra.c > > > @@ -253,11 +253,13 @@ static int tegra_cpuidle_enter(struct cpuidle_device *dev, > > > return err ? -1 : index; > > > } > > > > > > -static void tegra114_enter_s2idle(struct cpuidle_device *dev, > > > - struct cpuidle_driver *drv, > > > - int index) > > > +static int tegra114_enter_s2idle(struct cpuidle_device *dev, > > > + struct cpuidle_driver *drv, > > > + int index) > > > { > > > tegra_cpuidle_enter(dev, drv, index); > > > + > > > + return 0; > > > } > > > > > > /* > > > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > > > index f449584..b178da3 100644 > > > --- a/drivers/idle/intel_idle.c > > > +++ b/drivers/idle/intel_idle.c > > > @@ -175,13 +175,15 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev, > > > * Invoked as a suspend-to-idle callback routine with frozen user space, frozen > > > * scheduler tick and suspended scheduler clock on the target CPU. > > > */ > > > -static __cpuidle void intel_idle_s2idle(struct cpuidle_device *dev, > > > - struct cpuidle_driver *drv, int index) > > > +static __cpuidle int intel_idle_s2idle(struct cpuidle_device *dev, > > > + struct cpuidle_driver *drv, int index) > > > { > > > unsigned long eax = flg2MWAIT(drv->states[index].flags); > > > unsigned long ecx = 1; /* break on interrupt flag */ > > > > > > mwait_idle_with_hints(eax, ecx); > > > + > > > + return 0; > > > } > > > > > > /* > > > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h > > > index ec2ef63..bee10c0 100644 > > > --- a/include/linux/cpuidle.h > > > +++ b/include/linux/cpuidle.h > > > @@ -66,9 +66,9 @@ struct cpuidle_state { > > > * suspended, so it must not re-enable interrupts at any point (even > > > * temporarily) or attempt to change states of clock event devices. > > > */ > > > - void (*enter_s2idle) (struct cpuidle_device *dev, > > > - struct cpuidle_driver *drv, > > > - int index); > > > + int (*enter_s2idle)(struct cpuidle_device *dev, > > > + struct cpuidle_driver *drv, > > > + int index); > > > }; > > > > > > /* Idle State Flags */ > > > -- > > > 1.7.9.5 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel