[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMqyJG2UKG_P5Dbp6t=K98HAhBVHu-iuCrTjUx+NqzUJHTDA0w@mail.gmail.com>
Date: Tue, 5 Nov 2024 16:05:40 +0800
From: En-Wei WU <en-wei.wu@...onical.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Peter Seiderer <ps.report@....net>, steffen.klassert@...unet.com,
herbert@...dor.apana.org.au, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
kai.heng.feng@...onical.com, chia-lin.kao@...onical.com,
anthony.wong@...onical.com, kuan-ying.lee@...onical.com,
chris.chiu@...onical.com
Subject: Re: [PATCH ipsec v2] xfrm: check MAC header is shown with both
skb->mac_len and skb_mac_header_was_set()
Hi,
Can I kindly ask if there is any progress?
Thanks,
Regards.
On Fri, 18 Oct 2024 at 21:21, En-Wei WU <en-wei.wu@...onical.com> wrote:
>
> > Seems like the __netif_receive_skb_core() and dev_gro_receive() are
> > the places where it calls skb_reset_mac_len() with skb->mac_header =
> > ~0U.
> I believe it's the root cause.
>
> My concern is that if we put something like:
> + if (!skb_mac_header_was_set(skb)) {
> + DEBUG_NET_WARN_ON_ONCE(1);
> + skb->mac_len = 0;
> in skb_reset_mac_len(), it may degrade the RX path a bit.
>
> Catching the bug in xfrm4_remove_tunnel_encap() and
> xfrm6_remove_tunnel_encap() (the original patch) is nice because it
> won't affect the systems which are not using the xfrm.
>
> Kind Regards,
> En-Wei.
>
> On Mon, 14 Oct 2024 at 22:06, En-Wei WU <en-wei.wu@...onical.com> wrote:
> >
> > Hi, sorry for the late reply.
> >
> > I've tested this debug patch (with CONFIG_DEBUG_NET=y) on my machine,
> > and the DEBUG_NET_WARN_ON_ONCE never got triggered.
> >
> > Thanks.
> >
> > On Wed, 2 Oct 2024 at 14:59, Eric Dumazet <edumazet@...gle.com> wrote:
> > >
> > > On Wed, Oct 2, 2024 at 12:40 PM En-Wei WU <en-wei.wu@...onical.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > I would kindly ask if there is any progress :)
> > >
> > > Can you now try this debug patch (with CONFIG_DEBUG_NET=y ) :
> > >
> > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > index 39f1d16f362887821caa022464695c4045461493..e0e4154cbeb90474d92634d505869526c566f132
> > > 100644
> > > --- a/include/linux/skbuff.h
> > > +++ b/include/linux/skbuff.h
> > > @@ -2909,9 +2909,19 @@ static inline void
> > > skb_reset_inner_headers(struct sk_buff *skb)
> > > skb->inner_transport_header = skb->transport_header;
> > > }
> > >
> > > +static inline int skb_mac_header_was_set(const struct sk_buff *skb)
> > > +{
> > > + return skb->mac_header != (typeof(skb->mac_header))~0U;
> > > +}
> > > +
> > > static inline void skb_reset_mac_len(struct sk_buff *skb)
> > > {
> > > - skb->mac_len = skb->network_header - skb->mac_header;
> > > + if (!skb_mac_header_was_set(skb)) {
> > > + DEBUG_NET_WARN_ON_ONCE(1);
> > > + skb->mac_len = 0;
> > > + } else {
> > > + skb->mac_len = skb->network_header - skb->mac_header;
> > > + }
> > > }
> > >
> > > static inline unsigned char *skb_inner_transport_header(const struct sk_buff
> > > @@ -3014,11 +3024,6 @@ static inline void
> > > skb_set_network_header(struct sk_buff *skb, const int offset)
> > > skb->network_header += offset;
> > > }
> > >
> > > -static inline int skb_mac_header_was_set(const struct sk_buff *skb)
> > > -{
> > > - return skb->mac_header != (typeof(skb->mac_header))~0U;
> > > -}
> > > -
> > > static inline unsigned char *skb_mac_header(const struct sk_buff *skb)
> > > {
> > > DEBUG_NET_WARN_ON_ONCE(!skb_mac_header_was_set(skb));
> > > @@ -3043,6 +3048,7 @@ static inline void skb_unset_mac_header(struct
> > > sk_buff *skb)
> > >
> > > static inline void skb_reset_mac_header(struct sk_buff *skb)
> > > {
> > > + DEBUG_NET_WARN_ON_ONCE(skb->data < skb->head);
> > > skb->mac_header = skb->data - skb->head;
> > > }
> > >
> > > @@ -3050,6 +3056,7 @@ static inline void skb_set_mac_header(struct
> > > sk_buff *skb, const int offset)
> > > {
> > > skb_reset_mac_header(skb);
> > > skb->mac_header += offset;
> > > + DEBUG_NET_WARN_ON_ONCE(skb_mac_header(skb) < skb->head);
> > > }
> > >
> > > static inline void skb_pop_mac_header(struct sk_buff *skb)
Powered by blists - more mailing lists