netdev - Re: [PATCH] strparser: remove any offset before parsing messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180822183852.jnwlxnz54gbbf6po@davejwatson-mba.dhcp.thefacebook.com>
Date:   Wed, 22 Aug 2018 11:38:52 -0700
From:   Dave Watson <davejwatson@...com>
To:     Dominique Martinet <asmadeus@...ewreck.org>
CC:     Doron Roberts-Kedes <doronrk@...com>,
        Tom Herbert <tom@...ntonium.net>,
        "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] strparser: remove any offset before parsing messages

On 08/22/18 07:47 AM, Dominique Martinet wrote:
> > > As I wrote above, I think it should not be possible, so we're not
> > > even talking about a small percentage here.
> > > The reason I didn't use skb_pull (the head-only variant) is that I'd
> > > rather have the overhead than a BUG() if I'm wrong on this...
> > 
> > A printk in that section when (orig_offset + eaten > skb_headlen(head)) 
> > confirms that this case is not uncommon or impossible. Would have to do
> > more work to see how many hundreds of times per second, but it is not a
> > philosophical concern.
> 
> Hmm, right, it does happen if I force two bigger packets in a single
> write() on my reproducer; I guess my workload didn't exhibit that
> behaviour with a 9p client...
> 
> I've tried measuring that overhead as well by writing a more complex bpf
> program that would fetch the offset in the skb but for some reason I'm
> reading a 0 offset when it's not zero... well, not like there's much
> choice for this at this point anyway; I don't think we'll do this
> without pull, I'll put that on background.

For what it is worth we checked the offset in bpf, something
along the lines of
	  
struct kcm_rx_msg {   int full_len;  int offset;};
static inline struct kcm_rx_msg *kcm_rx_msg(struct __sk_buff *skb)
      { return (struct kcm_rx_msg *)skb->cb;}

int decode_framing(struct __sk_buff *skb)
{ return load_word(skb, kcm_rx_msg(skb)->offset);}

Although it did puzzle me for a while figuring that out when I ran in
to it.