On 11/23/24 10:42, Jinghao Jia wrote: > Under certain kernel configurations when building with Clang/LLVM, the > compiler does not generate a return or jump as the terminator > instruction for ip_vs_protocol_init(), triggering the following objtool > warning during build time: > > vmlinux.o: warning: objtool: ip_vs_protocol_init() falls through to next function __initstub__kmod_ip_vs_rr__935_123_ip_vs_rr_init6() > > At runtime, this either causes an oops when trying to load the ipvs > module or a boot-time panic if ipvs is built-in. This same issue has > been reported by the Intel kernel test robot previously. > > Digging deeper into both LLVM and the kernel code reveals this to be a > undefined behavior problem. ip_vs_protocol_init() uses a on-stack buffer > of 64 chars to store the registered protocol names and leaves it > uninitialized after definition. The function calls strnlen() when > concatenating protocol names into the buffer. With CONFIG_FORTIFY_SOURCE > strnlen() performs an extra step to check whether the last byte of the > input char buffer is a null character (commit 3009f891bb9f ("fortify: > Allow strlen() and strnlen() to pass compile-time known lengths")). > This, together with possibly other configurations, cause the following > IR to be generated: > > define hidden i32 @ip_vs_protocol_init() local_unnamed_addr #5 section ".init.text" align 16 !kcfi_type !29 { > %1 = alloca [64 x i8], align 16 > ... > > 14: ; preds = %11 > %15 = getelementptr inbounds i8, ptr %1, i64 63 > %16 = load i8, ptr %15, align 1 > %17 = tail call i1 @llvm.is.constant.i8(i8 %16) > %18 = icmp eq i8 %16, 0 > %19 = select i1 %17, i1 %18, i1 false > br i1 %19, label %20, label %23 > > 20: ; preds = %14 > %21 = call i64 @strlen(ptr noundef nonnull dereferenceable(1) %1) #23 > ... > > 23: ; preds = %14, %11, %20 > %24 = call i64 @strnlen(ptr noundef nonnull dereferenceable(1) %1, i64 noundef 64) #24 > ... > } > > The above code calculates the address of the last char in the buffer > (value %15) and then loads from it (value %16). Because the buffer is > never initialized, the LLVM GVN pass marks value %16 as undefined: > > %13 = getelementptr inbounds i8, ptr %1, i64 63 > br i1 undef, label %14, label %17 > > This gives later passes (SCCP, in particular) more DCE opportunities by > propagating the undef value further, and eventually removes everything > after the load on the uninitialized stack location: > > define hidden i32 @ip_vs_protocol_init() local_unnamed_addr #0 section ".init.text" align 16 !kcfi_type !11 { > %1 = alloca [64 x i8], align 16 > ... > > 12: ; preds = %11 > %13 = getelementptr inbounds i8, ptr %1, i64 63 > unreachable > } > > In this way, the generated native code will just fall through to the > next function, as LLVM does not generate any code for the unreachable IR > instruction and leaves the function without a terminator. > > Zero the on-stack buffer to avoid this possible UB. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202402100205.PWXIz1ZK-lkp@xxxxxxxxx/ > Co-developed-by: Ruowen Qin <ruqin@xxxxxxxxxx> > Signed-off-by: Ruowen Qin <ruqin@xxxxxxxxxx> > Signed-off-by: Jinghao Jia <jinghao7@xxxxxxxxxxxx> @Pablo, @Simon, @Julian: recent ipvs patches landed either on the net(-next) trees or the netfiler trees according to a random (?) pattern. What is your preference here? Should such patches go via netfilter or net? Or something else. FTR, I *think* netfilter should be the preferable target, but I'm open to other options. Thanks, Paolo