Re: [PATCH net] net/sched: act_pedit: really ensure the skb is writable

[Paolo Abeni <pabeni@redhat.com>]

On Wed, 2022-05-04 at 07:47 -0700, Jakub Kicinski wrote:
> On Wed, 04 May 2022 10:52:59 +0200 Paolo Abeni wrote:
> > On Tue, 2022-05-03 at 16:10 -0400, Jamal Hadi Salim wrote:
> > > What was the tc pedit command that triggered this?  
> > 
> > From the mptcp self-tests, mptcp_join.sh:
> > 
> > tc -n $ns2 filter add dev ns2eth$i egress \
> > 		protocol ip prio 1000 \
> > 		handle 42 fw \
> > 		action pedit munge offset 148 u8 invert \
> > 		pipe csum tcp \
> > 		index 100 || exit 1
> > 
> > It's used to corrupt a packet so that TCP csum is still correct while
> > the MPTCP one is not.
> > 
> > The relevant part is that the touched offset is outside the skb head.
> > 
> > > Can we add it to tdc tests?  
> > 
> > What happens in the mptcp self-tests it that an almost simultaneous
> > mptcp-level reinjection on another device using the same cloned data
> > get unintentionally corrupted and we catch it - when it sporadically
> > happens - via the MPTCP mibs.
> > 
> > While we could add the above pedit command, but I fear that a
> > meaningful test for the issue addressed here not fit the tdc
> > infrastructure easily.
> 
> For testing stuff like this would it be possible to inject packets
> with no headers pulled and frags in pages we marked read-only?
> We can teach netdevsim to do it.

We additionally need to ensure that the crafted packets are cloned,
otherwise the current code is AFAICS fine. And at the point we likely
want to configure the packet layout (hdrs/address) created by
netdevsim. 

> Obviously not as a pre-requisite for this patch.

I agree it looks a bit out-of-scope here ;)

Paolo