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.6 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no 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 4C434C433EB for ; Thu, 23 Jul 2020 19:07:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2A59B206F4 for ; Thu, 23 Jul 2020 19:07:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="uJ/LXXd4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728017AbgGWTHd (ORCPT ); Thu, 23 Jul 2020 15:07:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36030 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728010AbgGWTHd (ORCPT ); Thu, 23 Jul 2020 15:07:33 -0400 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C81BAC0619E4 for ; Thu, 23 Jul 2020 12:07:32 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id x9so3004868plr.2 for ; Thu, 23 Jul 2020 12:07:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=uJ/LXXd4WW3t/V9uP1pvh2+w7xCfmHmmU2q2MX994uvmdMhyUpnBd6wtTkFWVeSiVn fni2Skynmv78aJ9rstIyOgTssb4uBzBQL6YPmxb5SzQaGZtm2sB0Ln7Qk9r6vt5a/SEE gYzQRW0K8FC5lX9W9+bz+gbIOgybElEHRuMxxBvv7T+TJKJnGfXHkIe/b2UFhfrjNnt8 hmvs/pCT8mt2wp6eGsvntxKg2m7zjG9IalAT3RXT7FCW3qu0zRZl84tHISI7JSF/JPwL M06pYkfoNbZamtdmUh5GU20hgxfU9YBtWQ50on6QDjz/h882N3ikL5uKYSp6UNdyl0GU 5IDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=ndJue7Tot/jdLJIir3AqDE3exinHJQ2IOQYeBAriPUieyF/n/RW0Wc2rerUawqtjja bgRvlms7fUAr9OmE/623G+8ce8h0Qf7W++0skZMWqkrt7hzwNJCg3fWM32KXlginBJ53 q38IHpkl7+FsgbHB80O9peEQIQ18BZsfPKe2UW30lxuzZjRZDCLtgZ3pWYR1wATvzysF Jh79aQGO+pyBHOuCb+TZg8keW4kt7neWxQ/2ycqWV4fHLpUa5aa7LdmSntflYeEIHCRN Dei6oOyOt/OeKd2cmWqNAbEZ/fxKbUp1Ohm5xzG0uhMZ2px5yQhdJN3uV0CMNtaKXMKD vv0A== X-Gm-Message-State: AOAM5329dZhNeeHjE8l4SyGAgNkHP7PoT4tUXhVJFbD7hR1qA5WluvAO SAYb6RJjdESGBL8pP5SvtqHZNQ== X-Google-Smtp-Source: ABdhPJyFY8SK2yrpGMIJYPZOm/XbUPFW09vVIwr7j2Oue2l2cEXybEXOCRDyJvKCGSvI32jjRGApEQ== X-Received: by 2002:a17:902:22:: with SMTP id 31mr4590106pla.120.1595531251889; Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Received: from google.com ([2620:15c:201:2:f693:9fff:fef4:1b6d]) by smtp.gmail.com with ESMTPSA id h6sm3716298pfo.123.2020.07.23.12.07.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Date: Thu, 23 Jul 2020 12:07:24 -0700 From: Sami Tolvanen To: Neal Liu Cc: "Rafael J. Wysocki" , "Rafael J. Wysocki" , Len Brown , Daniel Lezcano , Thierry Reding , Jonathan Hunter , Jacob Pan , Matthias Brugger , ACPI Devel Maling List , Linux PM , linux-tegra , Linux ARM , "moderated list:ARM/Mediatek SoC..." , lkml , wsd_upstream Subject: Re: [PATCH v2] cpuidle: change enter_s2idle() prototype Message-ID: <20200723190724.GA1339461@google.com> 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> <1595233294.8055.0.camel@mtkswgap22> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1595233294.8055.0.camel@mtkswgap22> Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Mon, Jul 20, 2020 at 04:21:34PM +0800, Neal Liu wrote: > 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. I think you should include a better explanation in the commit message. Perhaps something like this? init_state_node assigns the same callback function to both enter and enter_s2idle despite mismatching function types, which trips indirect call checking with Control-Flow Integrity (CFI). > > > > 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. Rafael, are you happy with the commit message documenting the reason, or would you prefer to also add a comment before enter_s2idle? Sami 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=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 C00EFC433EC for ; Thu, 23 Jul 2020 19:07:51 +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 8E692206F4 for ; Thu, 23 Jul 2020 19:07:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="vjBYvM56"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="uJ/LXXd4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8E692206F4 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=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:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=eNEX5H/M7ScDi9bAY6jd4Khgy1gDlAFZFb+dYuNOJ9A=; b=vjBYvM563oqacXiMYViDVAsTC /+Kq1FhFBDO+GEcvHpcto/Nb9lndOCLuKJQNWmVd4O1279E6BLlst0ScZX5XrXvYxIAQxqa7AYQZt Z38nLYM4m9aJdXABUEvkYUl+vaNHVIvqckJ7k80ORL2ebgJp4QXClJDYu24tqn0tb0ePTYIvKO1lY 0AvbjHduRuJ7VPLWchZtFFDsBfJxrjAJ0Et2/3jqG0o6BkLTnxmQlOiGqXMdbUY2d8Bb2gUqwxi1e DOqLXn+O7SpM0pnMddqrbt13/n//u87UKzBd2MLKei7tc1dTLj6SEZPgJ+qoWXYKpu1xs04RcY2B5 qg6h36hzg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jygZ9-0003ii-RB; Thu, 23 Jul 2020 19:07:39 +0000 Received: from mail-pj1-x1043.google.com ([2607:f8b0:4864:20::1043]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jygZ6-0003hU-DS for linux-mediatek@lists.infradead.org; Thu, 23 Jul 2020 19:07:37 +0000 Received: by mail-pj1-x1043.google.com with SMTP id a9so3624805pjd.3 for ; Thu, 23 Jul 2020 12:07:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=uJ/LXXd4WW3t/V9uP1pvh2+w7xCfmHmmU2q2MX994uvmdMhyUpnBd6wtTkFWVeSiVn fni2Skynmv78aJ9rstIyOgTssb4uBzBQL6YPmxb5SzQaGZtm2sB0Ln7Qk9r6vt5a/SEE gYzQRW0K8FC5lX9W9+bz+gbIOgybElEHRuMxxBvv7T+TJKJnGfXHkIe/b2UFhfrjNnt8 hmvs/pCT8mt2wp6eGsvntxKg2m7zjG9IalAT3RXT7FCW3qu0zRZl84tHISI7JSF/JPwL M06pYkfoNbZamtdmUh5GU20hgxfU9YBtWQ50on6QDjz/h882N3ikL5uKYSp6UNdyl0GU 5IDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=Jl37NF06/lqQXGD1ue7KKkDucP/7zIlFJ1+tf1xgVXUTnknyN6Liyh+d4ZtqWAzYCv 0Me/FenogzD9X54GgmvX7kZGaCtJoE7IK/kAPyq+MoA/wFrJGKEV44aYqf3eG4Xi5U94 0HIKE3F/Gjw9fybHqNiNXhij0BHcmqzKzMtOgJot+EoGmv+0cL/6J4PPjMz96En/bP6B pLEVOqKl+dgaWv36znViBSxQlujJECZ8mUm2YR00j05YZk0zCy0IB/Rb0sQ0/qIKAHJJ nbOM/3Tg0SW8ItZgp6Mf8R/uGx0Etwc60NL8evLEbLpibAuLWUEeBIR9cnYY5Ho570nG vXCw== X-Gm-Message-State: AOAM53268muaLgeG3wQp7rhIrH29QP9j6HhmDbQd7ilZEGw0c+59RGpn z8ooQdeiHBsWcOM9/zfpmj540A== X-Google-Smtp-Source: ABdhPJyFY8SK2yrpGMIJYPZOm/XbUPFW09vVIwr7j2Oue2l2cEXybEXOCRDyJvKCGSvI32jjRGApEQ== X-Received: by 2002:a17:902:22:: with SMTP id 31mr4590106pla.120.1595531251889; Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Received: from google.com ([2620:15c:201:2:f693:9fff:fef4:1b6d]) by smtp.gmail.com with ESMTPSA id h6sm3716298pfo.123.2020.07.23.12.07.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Date: Thu, 23 Jul 2020 12:07:24 -0700 From: Sami Tolvanen To: Neal Liu Subject: Re: [PATCH v2] cpuidle: change enter_s2idle() prototype Message-ID: <20200723190724.GA1339461@google.com> 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> <1595233294.8055.0.camel@mtkswgap22> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1595233294.8055.0.camel@mtkswgap22> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200723_150736_549711_0CA7F59C X-CRM114-Status: GOOD ( 26.83 ) X-BeenThere: linux-mediatek@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 , "Rafael J. Wysocki" , Linux PM , Daniel Lezcano , "Rafael J. Wysocki" , lkml , Jonathan Hunter , ACPI Devel Maling List , Thierry Reding , "moderated list:ARM/Mediatek SoC..." , Matthias Brugger , Linux ARM , Len Brown Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Mon, Jul 20, 2020 at 04:21:34PM +0800, Neal Liu wrote: > 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. I think you should include a better explanation in the commit message. Perhaps something like this? init_state_node assigns the same callback function to both enter and enter_s2idle despite mismatching function types, which trips indirect call checking with Control-Flow Integrity (CFI). > > > > 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. Rafael, are you happy with the commit message documenting the reason, or would you prefer to also add a comment before enter_s2idle? Sami _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sami Tolvanen Subject: Re: [PATCH v2] cpuidle: change enter_s2idle() prototype Date: Thu, 23 Jul 2020 12:07:24 -0700 Message-ID: <20200723190724.GA1339461@google.com> 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> <1595233294.8055.0.camel@mtkswgap22> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1595233294.8055.0.camel@mtkswgap22> Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Neal Liu Cc: "Rafael J. Wysocki" , "Rafael J. Wysocki" , Len Brown , Daniel Lezcano , Thierry Reding , Jonathan Hunter , Jacob Pan , Matthias Brugger , ACPI Devel Maling List , Linux PM , linux-tegra , Linux ARM , "moderated list:ARM/Mediatek SoC..." , lkml , wsd_upstream List-Id: linux-tegra@vger.kernel.org On Mon, Jul 20, 2020 at 04:21:34PM +0800, Neal Liu wrote: > 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. I think you should include a better explanation in the commit message. Perhaps something like this? init_state_node assigns the same callback function to both enter and enter_s2idle despite mismatching function types, which trips indirect call checking with Control-Flow Integrity (CFI). > > > > 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. Rafael, are you happy with the commit message documenting the reason, or would you prefer to also add a comment before enter_s2idle? Sami 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=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 2902EC433E3 for ; Thu, 23 Jul 2020 19:09:14 +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 E876F20709 for ; Thu, 23 Jul 2020 19:09: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="JfS/1y8o"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="uJ/LXXd4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E876F20709 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.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:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=/RFVn7zBfH8kgY4MihHPe1VMoUthRo5J99nVLot4pNI=; b=JfS/1y8otW6wuRJih/hgZZoip zojmfjJ4ZmsvDg+POLSuzCykbuSejGfB8SLKfIHcu9jjdOB83Ab8BSSIdVIM9PrNDWeAfmRNZPU3S L1LANlGi7PnIAzvcHuTibZu4BkmKmRLBEeRRG2BbSUH3yYqlEuIbj0Z+87/CSmTzLxz2z4OJ2aKCn 0yMmV42vr0zDC+DvkYdoL5+hlXB/d8z7nNF0o7hb+ven9gbtVZpzttV4BMgnNf22tksVejw1Vuhv+ +RWWHbECsaIPV0J4xlkoZVDIkE6tb6781Aq62tiYxDTVnhwIKwR77ANEt5MYdyQ1DY1u3bOWYsHFA Qa943+zyw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jygZB-0003iu-7u; Thu, 23 Jul 2020 19:07:41 +0000 Received: from mail-pj1-x1044.google.com ([2607:f8b0:4864:20::1044]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jygZ6-0003hS-FY for linux-arm-kernel@lists.infradead.org; Thu, 23 Jul 2020 19:07:37 +0000 Received: by mail-pj1-x1044.google.com with SMTP id f16so3549957pjt.0 for ; Thu, 23 Jul 2020 12:07:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=uJ/LXXd4WW3t/V9uP1pvh2+w7xCfmHmmU2q2MX994uvmdMhyUpnBd6wtTkFWVeSiVn fni2Skynmv78aJ9rstIyOgTssb4uBzBQL6YPmxb5SzQaGZtm2sB0Ln7Qk9r6vt5a/SEE gYzQRW0K8FC5lX9W9+bz+gbIOgybElEHRuMxxBvv7T+TJKJnGfXHkIe/b2UFhfrjNnt8 hmvs/pCT8mt2wp6eGsvntxKg2m7zjG9IalAT3RXT7FCW3qu0zRZl84tHISI7JSF/JPwL M06pYkfoNbZamtdmUh5GU20hgxfU9YBtWQ50on6QDjz/h882N3ikL5uKYSp6UNdyl0GU 5IDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fmy4tO1tW/yB2p+/DBGuPfpuvcNIPQV/6jKnfwUymaM=; b=QkzKHQ6S+K8XwpDfpuzh2euCAC2C1+M5mvkljEhy1HlX9qyBHcUybTkS01irJwv56H 4xjXZmX68ysXvu75CTxyxo9YGwXidH3yhNOxnvXfPjzRhppCg+dcaecnFXR9+x1WwNU2 Sx97QXA12+mM29qvBXnnxN+t1FpK/J4UVImt0gEm7h4uDGUXesDAsunJwY2+M5KZrN57 n9dMrfWsg1QlAlCReMXax5a4dMxnpGKhe3Zik+LLMQNOsSS8tuq5OfGTp0VtVHQXadPp 5MTxmE/0+BJok+KhORp2OnmagBsjd8gzCSPa0B4Ef1Ff42Nb0FjpRsNb+xyBYdXsMUxE oXAg== X-Gm-Message-State: AOAM532Wy3C0NIPTvO493WZrX6eYVqtksMrWqdEf1y7iMJrybeqB3eo0 +iA7XO6V42Ez22Nj00Xgb3Z9aw== X-Google-Smtp-Source: ABdhPJyFY8SK2yrpGMIJYPZOm/XbUPFW09vVIwr7j2Oue2l2cEXybEXOCRDyJvKCGSvI32jjRGApEQ== X-Received: by 2002:a17:902:22:: with SMTP id 31mr4590106pla.120.1595531251889; Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Received: from google.com ([2620:15c:201:2:f693:9fff:fef4:1b6d]) by smtp.gmail.com with ESMTPSA id h6sm3716298pfo.123.2020.07.23.12.07.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2020 12:07:31 -0700 (PDT) Date: Thu, 23 Jul 2020 12:07:24 -0700 From: Sami Tolvanen To: Neal Liu Subject: Re: [PATCH v2] cpuidle: change enter_s2idle() prototype Message-ID: <20200723190724.GA1339461@google.com> 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> <1595233294.8055.0.camel@mtkswgap22> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1595233294.8055.0.camel@mtkswgap22> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200723_150736_586242_DFD9B4E6 X-CRM114-Status: GOOD ( 28.31 ) 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 , "Rafael J. Wysocki" , Linux PM , Daniel Lezcano , "Rafael J. Wysocki" , lkml , Jonathan Hunter , ACPI Devel Maling List , Thierry Reding , "moderated list:ARM/Mediatek SoC..." , Matthias Brugger , 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 On Mon, Jul 20, 2020 at 04:21:34PM +0800, Neal Liu wrote: > 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. I think you should include a better explanation in the commit message. Perhaps something like this? init_state_node assigns the same callback function to both enter and enter_s2idle despite mismatching function types, which trips indirect call checking with Control-Flow Integrity (CFI). > > > > 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. Rafael, are you happy with the commit message documenting the reason, or would you prefer to also add a comment before enter_s2idle? Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel