Re: [PATCH net-next] net: warn if mac header was not set

[Paolo Abeni <pabeni@redhat.com>]

Hello,

On Mon, 2022-06-20 at 02:30 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Make sure skb_mac_header(), skb_mac_offset() and skb_mac_header_len() uses
> are not fooled if the mac header has not been set.
> 
> These checks are enabled if CONFIG_DEBUG_NET=y
> 
> This commit will likely expose existing bugs in linux networking stacks.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/linux/skbuff.h | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 82edf0359ab32d0d7cd583676b38d569ce0b24cc..cd4a8268894acce4bde16dc0fedb7eb13706f515 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2763,8 +2763,14 @@ 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));
>  	return skb->head + skb->mac_header;
>  }
>  
> @@ -2775,14 +2781,10 @@ static inline int skb_mac_offset(const struct sk_buff *skb)
>  
>  static inline u32 skb_mac_header_len(const struct sk_buff *skb)
>  {
> +	DEBUG_NET_WARN_ON_ONCE(!skb_mac_header_was_set(skb));

The patch LGTM. 

I'm wondering if it's worthy adding similar debug checks for network
and transport offset (with more patches). e.g. still in
skb_mac_header_len():

	DEBUG_NET_WARN_ON_ONCE(skb->network_header < skb->mac_header);

Thanks!

Paolo