Ido Schimmel <ido...@idosch.org> wrote: > On Thu, Oct 29, 2020 at 05:36:19PM +0000, Aleksandr Nogikh wrote: > > From: Aleksandr Nogikh <nog...@google.com> > > > > 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 <nog...@google.com> > > Acked-by: Willem de Bruijn <will...@google.com> > > [...] > > > @@ -249,6 +249,9 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t > > gfp_mask, > > > > fclones->skb2.fclone = SKB_FCLONE_CLONE; > > } > > + > > + skb_set_kcov_handle(skb, kcov_common_handle()); > > Hi, > > This causes skb extensions to be allocated for the allocated skb, but > there are instances that blindly overwrite 'skb->extensions' by invoking > skb_copy_header() after __alloc_skb(). For example, skb_copy(), > __pskb_copy_fclone() and skb_copy_expand(). This results in the skb > extensions being leaked [1].
[..] > Other suggestions? Aleksandr, why was this made into an skb extension in the first place? AFAIU this feature is usually always disabled at build time. For debug builds (test farm /debug kernel etc) its always needed. If thats the case this u64 should be an sk_buff member, not an extension.
