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 03E51C4332F for ; Wed, 2 Nov 2022 20:24:53 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=tutSpURLdmzga1oj5G7WSAKEX8sojEvDGg4A3JGywPk=; b=3a4WTEKLj1u2eo jyaexwmSLxjQcxqD56z9K88IrxWU6xpmYh5ozwl9V1ItphCwqHU/myxvjxm2IPykJlNlJ1GoUFzJR lfDQcpSUB4d0QOmXyXuq9ENepZxmLIIXFZe9MORwpkhOQ86W8asNKjShQ3VLKiSBkTnka1EWsOY1E JXdnrNuck4rbLsBBiLgXzyDwCT70/WktmyJtCVLGJNS553XQjsWFNzH30VxY3UCbp5HPpVC8mP0z6 mt4FlcFbcXDuwxmyGoKV2eGgpTbq926CWpVAmrdb24vGNfFrSgrq2PzzQ8W/C3ifUyinH4eHkwypd 3awcFCfiKo4iR1vSpZiw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqKHG-00EEob-J6; Wed, 02 Nov 2022 20:23:58 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqKHD-00EEle-Hn for linux-arm-kernel@lists.infradead.org; Wed, 02 Nov 2022 20:23:56 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CD25461BF7; Wed, 2 Nov 2022 20:23:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 31530C433C1; Wed, 2 Nov 2022 20:23:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1667420634; bh=SoPxowNL2wQMY8yb7AJeEyGxnbtrkZim847pUvBQbwE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LfjPqEtYXiCdkBLiD6Mb9K3XA51IVPUzGRUTvms6p8otl6PuFvxWJt1eNfolnsfMh 8dOIiVYrNAvWUbc0eNesbhfxuTxND9PyOfr87g31Ntyy1O1D+CTiL0Ie98zFKhuL6L pQ6P5NPg9O9EYdY94YsL2yXqg36H/b9HAt0cdMS0OgSSvy64YmI0vtfYhObAcu3zvz dNfsvy2LS2sTnLtx58wwVQmNFpMeTJG42JD4GtztqHso9Ufxk1sH8tTONFs9g9eRjR Xm+yRgTchmBGh4Sk8fbw4WKFDXV+G1cCqTlkngWQBtrxjrVSY5vsGgYa9I9w0HKA3v fRU/uLjxEBNqQ== Date: Wed, 2 Nov 2022 13:23:51 -0700 From: Nathan Chancellor To: Kees Cook Cc: William Breathitt Gray , linux-iio@vger.kernel.org, Nick Desaulniers , Tom Rix , Sami Tolvanen , llvm@lists.linux.dev, linux-kernel@vger.kernel.org, patches@lists.linux.dev, Patrick Havelange , Jarkko Nikula , Oleksij Rempel , Pengutronix Kernel Team , Fabrice Gasnier , Vignesh Raghavendra , Julien Panis , David Lechner , linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-omap@vger.kernel.org Subject: Re: [PATCH 1/4] counter: Adjust final parameter type in function and signal callbacks Message-ID: References: <20221102172217.2860740-1-nathan@kernel.org> <202211021216.FF49E84C69@keescook> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <202211021216.FF49E84C69@keescook> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221102_132355_693688_F9A0BB65 X-CRM114-Status: GOOD ( 24.06 ) 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 Wed, Nov 02, 2022 at 12:21:23PM -0700, Kees Cook wrote: > On Wed, Nov 02, 2022 at 10:22:14AM -0700, Nathan Chancellor wrote: > > The ->signal_u32_read(), ->count_u32_read(), and ->count_u32_write() > > callbacks in 'struct counter_comp' expect the final parameter to have a > > type of 'u32' or 'u32 *' but the ops functions that are being assigned > > to those callbacks have an enumerated type as the final parameter. While > > these are compatible from an ABI perspective, they will fail the > > aforementioned CFI checks. > > > > Adjust the type of the final parameter in the ->signal_read(), > > ->function_read(), and ->function_write() callbacks in 'struct > > counter_ops' and their implementations to match the prototypes in > > 'struct counter_comp' to clear up these warnings and CFI failures. > > I don't understand these changes. Where do 'struct counter_comp' > and 'struct counter_ops' get confused? I can only find matching > ops/assignments/calls, so I must be missing something. This looks like > a loss of CFI granularity instead of having wrappers added if there is > an enum/u32 conversion needed somewhere. Right, I am not the biggest fan of this change myself and it is entirely possible that I am misreading the warnings from the commit message but I do not see how comp_node.comp.signal_u32_read = counter->ops->signal_read; and comp_node.comp.count_u32_read = counter->ops->function_read; in counter_add_watch(), comp.signal_u32_read = counter->ops->signal_read; in counter_signal_attrs_create(), and comp.count_u32_read = counter->ops->function_read; comp.count_u32_write = counter->ops->function_write; in counter_count_attrs_create() are currently safe under kCFI, since the final parameter type of the prototypes in 'struct counter_ops' does not match the final parameter type of the prototypes in 'struct counter_comp'. I would expect the indirect calls in counter_get_data() and counter_comp_u32_show() to fail currently. I briefly looked at making the 'struct counter_comp' callbacks match the 'struct counter_ops' ones but the COUNTER_COMP macros in include/linux/counter.h made it seem like these callbacks might be used by implementations that might use different enumerated types as the final parameter. I can look a little closer to see if we can make everything match. I am not sure how wrappers would work here, I can take a look into how feasible that is. Cheers, Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel