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 5C337C4332F for ; Wed, 2 Nov 2022 21:31:15 +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-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-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=I44kW4gI2IfV0TYt0wFJv0G1Qo2WDy7kwLir+NPpTko=; b=BmHcjXJ+nugD3uisBlJ+buJ043 ef7Jc4WdDR6is7kyPCBuFezp65o4rCkbw1dcqNXB45Tmj5ibka7CNahjJuudbyoibY07yRZ5foLVr FTceKKhDDaPAenfcTrz+mAfb/sZEXAmKm6cf4Q48EzUJzqqcDTgGCrHyrMM08JcZ7Z8x9zIeLj/VG EtEJen6sLlYEbe74Q5JHgeiXMjcDVTCTl/gFBJR5no0rPxKMkn6pPpqsWZcRgMrQV8Tlk8rTREAkl 8z62NLTaLoJLpaA8X0+d/Y2Nrq/j09jcVIMHfqWzOuFVjRF7AKbU/8hrn067c2f/wvNGyiv+zJNKe C5MW9UHA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqLJO-00EVrZ-Uw; Wed, 02 Nov 2022 21:30:15 +0000 Received: from mail-qv1-xf33.google.com ([2607:f8b0:4864:20::f33]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqLJL-00EVqc-DJ for linux-arm-kernel@lists.infradead.org; Wed, 02 Nov 2022 21:30:12 +0000 Received: by mail-qv1-xf33.google.com with SMTP id o8so13394490qvw.5 for ; Wed, 02 Nov 2022 14:30:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=NnhizWAIbi5qruvOMfkQD7JHf83kJnMNlqaH+GW5Vh0=; b=QDdBEz1O25l6wo9j8kV5Sjmnbo1c3FXDV4OHpi1WeilJLJ5Z+VOzqW0FQ5cxaWwPp+ rrfQWNepZUUXSyKd1X8Hu8Eqi0blhQGKXmerRF4DuU6AQ3b/jqUcMkOyfVbsPhYnGKLs 1nK2/BEq403NTL0VW9SFzDitWTyJmAlcXR/QlT7KsSNC/xqEoTxG5VdewkDFCIp9q+K1 h81ZawANVfE0Ip2sP3p34JTqN+d1FE9G7pXbs7cO2NGWjJYZuvrLuwgNjpuhsOzOsXKw qm82J9JF8p5KKnkvgDteUXWfixnKplnTEYOL2UgFW1Vv4P3WapBiGCT5xFdPB4ZdnzJ1 T9sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=NnhizWAIbi5qruvOMfkQD7JHf83kJnMNlqaH+GW5Vh0=; b=zpMpG8K/z7R2cTOd3db6h7O0dJonRfH+DihNrWixjOlkVmTFNO2gITbyHKUeX04iCF ffHy93INPgRfESoqdHIMMyH7mG80kZJATIb0FygvYUz9+zJeKUvb73SmX492ZHqGX5yX OI9RQY9l3FcnGsb1bea3op/jVNJE48/TAyAJ3KnfB4ZLKQWdySxTJfruftXDMJ9YcZix +nVzy2TGmhJrFFDwh6k7OJD8EzoEkPxXrBjI3U1Smyj2BBGUXLewCAl1g3YcZzkkJgKi Kqn5pUevYqxfDdUPgEH2D1iYa+3GTAL+kY6M2UvAbOQvf6pVvwk0k0LxN2dzdDDey6mF tyLA== X-Gm-Message-State: ACrzQf2d/BXXpFl5ptFeqwAQ6NCyTwQIQ4njuPqMaCSKkUeQPB82800C Ol4mcXsKIkg7R8IzuytObDfClw== X-Google-Smtp-Source: AMsMyM5b+7hwrgJkxmNhxObQT8swErvoEdNYJyoRn824mVM6oOxzwoYl3T/uGWd8STZXh4U9qyFLdw== X-Received: by 2002:a0c:9c8b:0:b0:4b1:ac82:5c50 with SMTP id i11-20020a0c9c8b000000b004b1ac825c50mr24036814qvf.15.1667424609463; Wed, 02 Nov 2022 14:30:09 -0700 (PDT) Received: from fedora (69-109-179-158.lightspeed.dybhfl.sbcglobal.net. [69.109.179.158]) by smtp.gmail.com with ESMTPSA id m11-20020ac8688b000000b0039a610a04b1sm7195595qtq.37.2022.11.02.14.30.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Nov 2022 14:30:08 -0700 (PDT) Date: Wed, 2 Nov 2022 17:30:06 -0400 From: William Breathitt Gray To: Nathan Chancellor Cc: Kees Cook , 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 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221102_143011_505030_5A1E602D X-CRM114-Status: GOOD ( 33.31 ) 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: multipart/mixed; boundary="===============4914322380361162724==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============4914322380361162724== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HEJ7wVOWhWkBpraW" Content-Disposition: inline --HEJ7wVOWhWkBpraW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 02, 2022 at 01:23:51PM -0700, Nathan Chancellor wrote: > 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. Wh= ile > > > these are compatible from an ABI perspective, they will fail the > > > aforementioned CFI checks. > > >=20 > > > 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. > >=20 > > 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. >=20 > 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 >=20 > comp_node.comp.signal_u32_read =3D counter->ops->signal_read; >=20 > and >=20 > comp_node.comp.count_u32_read =3D counter->ops->function_read; >=20 > in counter_add_watch(), >=20 > comp.signal_u32_read =3D counter->ops->signal_read; >=20 > in counter_signal_attrs_create(), and >=20 > comp.count_u32_read =3D counter->ops->function_read; > comp.count_u32_write =3D counter->ops->function_write; >=20 > 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. >=20 > 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. >=20 > I am not sure how wrappers would work here, I can take a look into how > feasible that is. >=20 > Cheers, > Nathan The intention of the code here is to treat the last parameter as an makeshift generic; the u32 will always be some corresponding enum type provided by the driver. The expectation is for drivers to define components via respective COUNTER_COMP_* macros, such that the assignments of the *_u32_read/*_u32_write callbacks are abstracted away and the driver can treat the respective last parameter as of the desired enum type. For example, COUNTER_COMP_DIRECTION is expected to be used with enum counter_count_direction, COUNTER_COMP_POLARITY is expected to be used with enum counter_signal_polarity, etc. What would be nice is if there is a way to ensure the enum type of the last parameter of the callback provided to these COUNTER_COMP_* macros matches the particular respective COUNTER_COMP_* macro's expectation; e.g. we should get some sort of error if COUNTER_COMP_DIRECTION is used for a enum counter_signal_level, etc. William Breathitt Gray --HEJ7wVOWhWkBpraW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQSNN83d4NIlKPjon7a1SFbKvhIjKwUCY2LhXgAKCRC1SFbKvhIj K7ujAP4vYYp4QiiMKB8y1V9TP6m+SGeLI3IIjMY/y3kpizOZ5gD/XNnop9AeiGkp N0Emw/FK2KiTf7jlxG8uhVJCVgkrKQ8= =e2Hi -----END PGP SIGNATURE----- --HEJ7wVOWhWkBpraW-- --===============4914322380361162724== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============4914322380361162724==--