Message-ID: <20240910-utopian-meticulous-dodo-4ec230-mkl@pengutronix.de>
Date: Tue, 10 Sep 2024 11:56:56 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Simon Horman <horms@...nel.org>, Nathan Chancellor <nathan@...nel.org>
Cc: kernel@...gutronix.de, Vincent Mailhol <mailhol.vincent@...adoo.fr>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Heiko Stuebner <heiko@...ech.de>, linux-can@...r.kernel.org, netdev@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, llvm@...ts.linux.dev, 
	patches@...ts.linux.dev
Subject: Re: [PATCH] can: rockchip_canfd: fix return type of
 rkcanfd_start_xmit()

On 09.09.2024 15:35:46, Simon Horman wrote:
> On Mon, Sep 09, 2024 at 10:57:06AM +0200, Marc Kleine-Budde wrote:
> > On 09.09.2024 09:44:48, Simon Horman wrote:
> > > On Fri, Sep 06, 2024 at 01:26:41PM -0700, Nathan Chancellor wrote:
> > > > With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
> > > > indirect call targets are validated against the expected function
> > > > pointer prototype to make sure the call target is valid to help mitigate
> > > > ROP attacks. If they are not identical, there is a failure at run time,
> > > > which manifests as either a kernel panic or thread getting killed. A
> > > > warning in clang aims to catch these at compile time, which reveals:
> > > > 
> > > >   drivers/net/can/rockchip/rockchip_canfd-core.c:770:20: error: incompatible function pointer types initializing 'netdev_tx_t (*)(struct sk_buff *, struct net_device *)' (aka 'enum netdev_tx (*)(struct sk_buff *, struct net_device *)') with an expression of type 'int (struct sk_buff *, struct net_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
> > > >     770 |         .ndo_start_xmit = rkcanfd_start_xmit,
> > > >         |                           ^~~~~~~~~~~~~~~~~~
> > > > 
> > > > ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
> > > > 'netdev_tx_t', not 'int' (although the types are ABI compatible). Adjust
> > > > the return type of rkcanfd_start_xmit() to match the prototype's to
> > > > resolve the warning.
> > > > 
> > > > Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
> > > > Signed-off-by: Nathan Chancellor <nathan@...nel.org>
> > > 
> > > Thanks, I was able to reproduce this problem at build time
> > > and that your patch addresses it.
> > 
> > FTR: the default clang in Debian unstable, clang-16.0.6 doesn't support
> > this. With clang-20 from experimental it works, haven't checked older
> > versions, though.
> 
> FTR: I checked using 18.1.8 from here [1][2].
> 
> [1] https://mirrors.edge.kernel.org/pub/tools/llvm/
> [2] https://mirrors.edge.kernel.org/pub/tools/llvm/files/

I was a bit hasty yesterday, clang-20 and W=1 produces these errors:

| include/linux/vmstat.h:517:36: error: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Werror,-Wenum-enum-conversion]
|   517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
|       |                               ~~~~~~~~~~~ ^ ~~~
| 1 error generated.

However I fail to reproduce the ndo_start_xmit problem. Even with 18.1.8
from kernel.org.


The following command (ARCH is unset, compiling x86 -> x86) produces the
above shown "vmstat.h" problems....

| $ make LLVM=1 LLVM_IAS=1 LLVM_SUFFIX=-20 drivers/net/can/rockchip/  W=1 CONFIG_WERROR=0

... but not the ndo_start_xmit problem.


Am I missing a vital .config option?

| $ grep "CLANG\|CFI" .config
| CONFIG_CC_IS_CLANG=y
| CONFIG_CLANG_VERSION=200000
| CONFIG_CFI_AUTO_DEFAULT=y
| CONFIG_FUNCTION_PADDING_CFI=11
| CONFIG_LTO_CLANG=y
| CONFIG_ARCH_SUPPORTS_LTO_CLANG=y
| CONFIG_ARCH_SUPPORTS_LTO_CLANG_THIN=y
| CONFIG_HAS_LTO_CLANG=y
| CONFIG_LTO_CLANG_THIN=y
| CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
| CONFIG_ARCH_USES_CFI_TRAPS=y
| CONFIG_CFI_CLANG=y
| # CONFIG_CFI_PERMISSIVE is not set


V=1 gives this output:

|   clang-20 -Wp,-MMD,drivers/net/can/rockchip/.rockchip_canfd-core.o.d
|   -nostdinc -Iarch/x86/include -I./arch/x86/include/generated
|   -Iinclude -I./include -Iarch/x86/include/uapi
|   -I./arch/x86/include/generated/uapi -Iinclude/uapi
|   -I./include/generated/uapi -include include/linux/compiler-version.h
|   -include include/linux/kconfig.h -include
|   include/linux/compiler_types.h -D__KERNEL__
|   --target=x86_64-linux-gnu -fintegrated-as
|   -Werror=unknown-warning-option -Werror=ignored-optimization-argument
|   -Werror=option-ignored -Werror=unused-command-line-argument
|   -fmacro-prefix-map== -Wundef -DKBUILD_EXTRA_WARN1 -std=gnu11
|   -fshort-wchar -funsigned-char -fno-common -fno-PIE
|   -fno-strict-aliasing -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx
|   -fcf-protection=branch -fno-jump-tables -m64 -falign-loops=1
|   -mno-80387 -mno-fp-ret-in-387 -mstack-alignment=8 -mskip-rax-setup
|   -mtune=generic -mno-red-zone -mcmodel=kernel -Wno-sign-compare
|   -fno-asynchronous-unwind-tables -mretpoline-external-thunk
|   -mindirect-branch-cs-prefix -mfunction-return=thunk-extern
|   -fpatchable-function-entry=11,11 -fno-delete-null-pointer-checks -O2
|   -fstack-protector-strong -fomit-frame-pointer
|   -fno-stack-clash-protection -fno-lto -flto=thin -fsplit-lto-unit
|   -fvisibility=hidden -fsanitize=kcfi -falign-functions=16
|   -fstrict-flex-arrays=3 -fno-strict-overflow -fno-stack-check -Wall
|   -Wundef -Werror=implicit-function-declaration -Werror=implicit-int
|   -Werror=return-type -Werror=strict-prototypes -Wno-format-security
|   -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member
|   -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=2048
|   -Wno-gnu -Wvla -Wno-pointer-sign -Wcast-function-type
|   -Wimplicit-fallthrough -Werror=date-time
|   -Werror=incompatible-pointer-types -Wenum-conversion -Wextra
|   -Wunused -Wmissing-format-attribute -Wmissing-include-dirs
|   -Wunused-const-variable -Wno-missing-field-initializers
|   -Wno-type-limits -Wno-shift-negative-value -Wno-sign-compare
|   -Wno-unused-parameter -g -gdwarf-5 -DDEBUG
|   -Idrivers/net/can/rockchip -Idrivers/net/can/rockchip -DMODULE
|   -DKBUILD_BASENAME='"rockchip_canfd_core"'
|   -DKBUILD_MODNAME='"rockchip_canfd"'
|   -D__KBUILD_MODNAME=kmod_rockchip_canfd -c -o
|   drivers/net/can/rockchip/rockchip_canfd-core.o
|   drivers/net/can/rockchip/rockchip_canfd-core.c
| In file included from drivers/net/can/rockchip/rockchip_canfd-core.c:25:
| In file included from drivers/net/can/rockchip/rockchip_canfd.h:11:
| In file included from include/linux/can/dev.h:18:
| In file included from include/linux/can/bittiming.h:9:
| In file included from include/linux/netdevice.h:38:
| In file included from include/net/net_namespace.h:43:
| In file included from include/linux/skbuff.h:17:
| In file included from include/linux/bvec.h:10:
| In file included from include/linux/highmem.h:8:
| In file included from include/linux/cacheflush.h:5:
| In file included from arch/x86/include/asm/cacheflush.h:5:
| In file included from include/linux/mm.h:2232:
| include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
|   517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
|       |                               ~~~~~~~~~~~ ^ ~~~
| 1 warning generated.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists