From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BD520381AE for ; Wed, 6 Dec 2023 15:36:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="gTruqnlu" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=34JGkeDw2fzo5aYJ5fytyFCmmA2EAJ4M9xy4pwn6wrM=; b=gTruqnlu8eDmXa3L0iyeSD2Nnn ePXXo7+GcN32qytdrXaWYSLANmme4ALZxhg0JQlNFwM/MH4cviy77G8UNNHVpyThWd9OSqLjvfyjH n+hvMI4zCb/iJgc+nB2YqllaE+P53evNS/9fuIg8TOPrU7vxPv0Z2HgxAq+kGffHXcdGVjW87Znxc j4vkPvTk0bS8qgVSzZs8fcJ4jZGSyPTxTSPz0Q2sEpN7Cn7GkfA4d44EOHD4gLC5FjiKQEaNcynfB DtD725Qv+e8+0LnA0+qRxiZ5Qcjpt7or8VjiV6p28m3HroSjoRF0w57Lro7i//DTiB2B6gCrZJ6mU cXJtqFlQ==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1rAtw5-0031PW-8d; Wed, 06 Dec 2023 15:35:41 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 36B6E300451; Wed, 6 Dec 2023 16:35:40 +0100 (CET) Date: Wed, 6 Dec 2023 16:35:40 +0100 From: Peter Zijlstra To: Alexei Starovoitov Cc: Jiri Olsa , Song Liu , Song Liu , Paul Walmsley , Palmer Dabbelt , Albert Ou , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , X86 ML , "H. Peter Anvin" , "David S. Miller" , David Ahern , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Arnd Bergmann , Sami Tolvanen , Kees Cook , Nathan Chancellor , Nick Desaulniers , linux-riscv , LKML , Network Development , bpf , linux-arch , clang-built-linux , Josh Poimboeuf , Joao Moreira , Mark Rutland Subject: Re: [PATCH v2 2/2] x86/cfi,bpf: Fix BPF JIT call Message-ID: <20231206153540.GA36423@noisy.programming.kicks-ass.net> References: <20231130133630.192490507@infradead.org> <20231130134204.136058029@infradead.org> <20231204091334.GM3818@noisy.programming.kicks-ass.net> <20231204111128.GV8262@noisy.programming.kicks-ass.net> <20231204125239.GA1319@noisy.programming.kicks-ass.net> <20231204181614.GA7299@noisy.programming.kicks-ass.net> <20231204183354.GC7299@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: llvm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Dec 04, 2023 at 05:18:31PM -0800, Alexei Starovoitov wrote: > How about > +int get_cfi_offset(void) > +{ > + switch (cfi_mode) { > + case CFI_FINEIBT: > + return 16; > + case CFI_KCFI: > +#ifdef CONFIG_CALL_PADDING > + return 16; > +#else > + return 5; > +#endif > + default: > + return 0; > + } > +} Yeah, that works. I'll go make it happen. > Separately we need to deal with bpf_for_each_array_elem() > which doesn't look easy. > And fix tcp_set_ca_state() as well (which is even harder). > > Just to see where places like these are I did: > +__nocfi > BPF_CALL_4(bpf_loop, u32, nr_loops, void *, callback_fn, void *, callback_ctx, > +__nocfi > static long bpf_for_each_hash_elem(struct bpf_map *map, > bpf_callback_t callback_fn, > +__nocfi > static enum hrtimer_restart bpf_timer_cb(struct hrtimer *hrtimer) > +__nocfi > static int __bpf_rbtree_add(struct bpf_rb_root *root, > +__nocfi > BPF_CALL_4(bpf_user_ringbuf_drain, struct bpf_map *, map, > +__nocfi > void tcp_set_ca_state(struct sock *sk, const u8 ca_state) > +__nocfi > void tcp_init_congestion_control(struct sock *sk) > +__nocfi > void tcp_enter_loss(struct sock *sk) > +__nocfi > static void tcp_cong_avoid(struct sock *sk, u32 ack, u32 acked) > +__nocfi > static inline void tcp_in_ack_event(struct sock *sk, u32 flags) > > and more... Which is clearly not a direction to go. > > Instead of annotating callers is there a way to say that > all bpf_callback_t calls are nocfi? Well, ideally they would all actually use CFI, I'll go figure out how all this works and think about it. Thanks! > I feel the patches scratched the iceberg. Yeah, clearly :/ I'll go stare at it all.