All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net, laforge@osmocom.org,
	pespin@sysmocom.de, osmith@sysmocom.de, kuba@kernel.org,
	pabeni@redhat.com, edumazet@google.com, fw@strlen.de
Subject: Re: [PATCH net-next 02/12] gtp: properly parse extension headers
Date: Fri, 26 Apr 2024 21:28:52 +0100	[thread overview]
Message-ID: <20240426202852.GD516117@kernel.org> (raw)
In-Reply-To: <20240425105138.1361098-3-pablo@netfilter.org>

On Thu, Apr 25, 2024 at 12:51:28PM +0200, Pablo Neira Ayuso wrote:
> Currently GTP packets are dropped if the next extension field is set to
> non-zero value, but this are valid GTP packets.
> 
> TS 29.281 provides a longer header format, which is defined as struct
> gtp1_header_long. Such long header format is used if any of the S, PN, E
> flags is set.
> 
> This long header is 4 bytes longer than struct gtp1_header, plus
> variable length (optional) extension headers. The next extension header
> field is zero is no extension header is provided.
> 
> The extension header is composed of a length field which includes total
> number of 4 byte words including the extension header itself (1 byte),
> payload (variable length) and next type (1 byte). The extension header
> size and its payload is aligned to 4 bytes.
> 
> A GTP packet might come with a chain extensions headers, which makes it
> slightly cumbersome to parse because the extension next header field
> comes at the end of the extension header, and there is a need to check
> if this field becomes zero to stop the extension header parser.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  drivers/net/gtp.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/net/gtp.h |  5 +++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 4680cdf4fa70..9451c74c1a7d 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -567,6 +567,43 @@ static int gtp1u_handle_echo_resp(struct gtp_dev *gtp, struct sk_buff *skb)
>  				       msg, 0, GTP_GENL_MCGRP, GFP_ATOMIC);
>  }
>  
> +static int gtp_parse_exthdrs(struct sk_buff *skb, unsigned int *hdrlen)
> +{
> +	struct gtp_ext_hdr *gtp_exthdr, _gtp_exthdr;
> +	unsigned int offset = *hdrlen;
> +	__u8 *next_type, _next_type;
> +
> +	/* From 29.060: "The Extension Header Length field specifies the length
> +	 * of the particular Extension header in 4 octets units."
> +	 *
> +	 * This length field includes length field size itself (1 byte),
> +	 * payload (variable length) and next type (1 byte). The extension
> +	 * header is aligned to to 4 bytes.
> +	 */
> +
> +	do {
> +		gtp_exthdr = skb_header_pointer(skb, offset, sizeof(gtp_exthdr),

Hi Pablo,

Should this be sizeof(*gtp_exthdr)?

And likewise, in the ip_version calculation in gtp_inner_proto()
in [PATCH 11/12] gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP 

Flagged by Coccinelle.

> +						&_gtp_exthdr);
> +		if (!gtp_exthdr || !gtp_exthdr->len)
> +			return -1;
> +
> +		offset += gtp_exthdr->len * 4;
> +
> +		/* From 29.060: "If no such Header follows, then the value of
> +		 * the Next Extension Header Type shall be 0."
> +		 */
> +		next_type = skb_header_pointer(skb, offset - 1,
> +					       sizeof(_next_type), &_next_type);
> +		if (!next_type)
> +			return -1;
> +
> +	} while (*next_type != 0);
> +
> +	*hdrlen = offset;
> +
> +	return 0;
> +}

...

  reply	other threads:[~2024-04-26 20:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25 10:51 [PATCH net-next 00/12] gtp updates for net-next (v2) Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 01/12] gtp: remove useless initialization Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 02/12] gtp: properly parse extension headers Pablo Neira Ayuso
2024-04-26 20:28   ` Simon Horman [this message]
2024-05-02 10:36     ` Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 03/12] gtp: prepare for IPv6 support Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 04/12] gtp: add " Pablo Neira Ayuso
2024-04-26 20:41   ` Simon Horman
2024-05-02 10:38     ` Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 05/12] gtp: use IPv6 address /64 prefix for UE/MS Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 06/12] gtp: pass up link local traffic to userspace socket Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 07/12] gtp: move debugging to skbuff build helper function Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 08/12] gtp: remove IPv4 and IPv6 header from context object Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 09/12] gtp: add helper function to build GTP packets from an IPv4 packet Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 10/12] gtp: add helper function to build GTP packets from an IPv6 packet Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 11/12] gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 12/12] gtp: identify tunnel via GTP device + GTP version + TEID + family Pablo Neira Ayuso
2024-04-26  2:29 ` [PATCH net-next 00/12] gtp updates for net-next (v2) Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2024-04-23 22:39 [PATCH net-next 00/12] GTP driver updates for net-next Pablo Neira Ayuso
2024-04-23 22:39 ` [PATCH net-next 02/12] gtp: properly parse extension headers Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240426202852.GD516117@kernel.org \
    --to=horms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=kuba@kernel.org \
    --cc=laforge@osmocom.org \
    --cc=netdev@vger.kernel.org \
    --cc=osmith@sysmocom.de \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    --cc=pespin@sysmocom.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.