Re: [PATCH v3 2/3] net: add kcov handle to skb extensions
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2020-10-26 16:58:11
Also in:
linux-wireless, lkml
On Mon, Oct 26, 2020 at 11:11 AM Aleksandr Nogikh [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Aleksandr Nogikh <redacted> Remote KCOV coverage collection enables coverage-guided fuzzing of the code that is not reachable during normal system call execution. It is especially helpful for fuzzing networking subsystems, where it is common to perform packet handling in separate work queues even for the packets that originated directly from the user space. Enable coverage-guided frame injection by adding kcov remote handle to skb extensions. Default initialization in __alloc_skb and __build_skb_around ensures that no socket buffer that was generated during a system call will be missed. Code that is of interest and that performs packet processing should be annotated with kcov_remote_start()/kcov_remote_stop(). An alternative approach is to determine kcov_handle solely on the basis of the device/interface that received the specific socket buffer. However, in this case it would be impossible to distinguish between packets that originated during normal background network processes or were intentionally injected from the user space. Signed-off-by: Aleksandr Nogikh <redacted> -- v2 -> v3: * Reimplemented this change. Now kcov handle is added to skb extensions instead of sk_buff. v1 -> v2: * Updated the commit message. --- include/linux/skbuff.h | 31 +++++++++++++++++++++++++++++++ net/core/skbuff.c | 11 +++++++++++ 2 files changed, 42 insertions(+)diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index a828cf99c521..b63d90faa8e9 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h@@ -4150,6 +4150,9 @@ enum skb_ext_id { #endif #if IS_ENABLED(CONFIG_MPTCP) SKB_EXT_MPTCP, +#endif +#if IS_ENABLED(CONFIG_KCOV) + SKB_EXT_KCOV_HANDLE, #endif SKB_EXT_NUM, /* must be last */ };@@ -4605,5 +4608,33 @@ static inline void skb_reset_redirect(struct sk_buff *skb) #endif } +#ifdef CONFIG_KCOV + +static inline void skb_set_kcov_handle(struct sk_buff *skb, const u64 kcov_handle) +{ + /* No reason to allocate skb extensions to set kcov_handle if kcov_handle is 0. */
If the handle does not need to be set if zero, why then set it if the skb has extensions?
+ if (skb_has_extensions(skb) || kcov_handle) {
+ u64 *kcov_handle_ptr = skb_ext_add(skb, SKB_EXT_KCOV_HANDLE);skb_ext_add and skb_ext_find are not defined unless CONFIG_SKB_EXTENSIONS. Perhaps CONFIG_KCOV should be made to select that?
+
+ if (kcov_handle_ptr)
+ *kcov_handle_ptr = kcov_handle;
+ }
+}
+
+static inline u64 skb_get_kcov_handle(struct sk_buff *skb)
+{
+ u64 *kcov_handle = skb_ext_find(skb, SKB_EXT_KCOV_HANDLE);
+
+ return kcov_handle ? *kcov_handle : 0;
+}
+
+#else
+
+static inline void skb_set_kcov_handle(struct sk_buff *skb, const u64 kcov_handle) { }
+
+static inline u64 skb_get_kcov_handle(struct sk_buff *skb) { return 0; }
+
+#endif /* CONFIG_KCOV */