linux-kernel - Re: [PATCH ipsec v2] xfrm: check MAC header is shown with both skb->mac_len and skb_mac_header_was

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: <CAMqyJG2MqU46jRC1NzYCUeJ45fiP5Z5nS78Mi0FLFjbKbLVrFg@mail.gmail.com>
Date: Mon, 14 Oct 2024 22:06:14 +0200
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, 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)