This patch introduces 'hop_tlv_hdr' and 'hop_calipso_hdr' structs, in
order to access fields in a readable way in "ip6_parse_tlv".
Signed-off-by: Richard Gobert <[email protected]>
---
include/net/ipv6.h | 16 ++++++++++++++++
net/ipv6/exthdrs.c | 30 +++++++++++++++++-------------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index cf25ea21d770..61677946ed46 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -151,6 +151,22 @@ struct frag_hdr {
__be32 identification;
};
+struct hop_tlv_hdr {
+ u8 tlv_type;
+ u8 tlv_len;
+};
+
+/* CALIPSO RFC 5570 */
+struct hop_calipso_hdr {
+ u8 tlv_type;
+ u8 tlv_len;
+ u32 domain_interpretation;
+ u8 cmpt_len;
+ u8 sens_lvl;
+ u16 checksum;
+ u64 cmpt_bitmap;
+} __packed;
+
/*
* Jumbo payload option, as described in RFC 2675 2.
*/
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 4952ae792450..5db624299da4 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -114,7 +114,7 @@ static bool ip6_parse_tlv(bool hopbyhop,
struct sk_buff *skb,
int max_count)
{
- int len = (skb_transport_header(skb)[1] + 1) << 3;
+ int len = ipv6_optlen((struct ipv6_opt_hdr *)skb_transport_header(skb));
const unsigned char *nh = skb_network_header(skb);
int off = skb_network_header_len(skb);
bool disallow_unknowns = false;
@@ -890,15 +890,16 @@ static inline struct net *ipv6_skb_net(struct sk_buff *skb)
static bool ipv6_hop_ra(struct sk_buff *skb, int optoff)
{
- const unsigned char *nh = skb_network_header(skb);
+ struct hop_tlv_hdr *tlv_hdr =
+ (struct hop_tlv_hdr *)(skb_network_header(skb) + optoff);
- if (nh[optoff + 1] == 2) {
+ if (tlv_hdr->tlv_len == 2) {
IP6CB(skb)->flags |= IP6SKB_ROUTERALERT;
- memcpy(&IP6CB(skb)->ra, nh + optoff + 2, sizeof(IP6CB(skb)->ra));
+ memcpy(&IP6CB(skb)->ra, tlv_hdr + 1, sizeof(IP6CB(skb)->ra));
return true;
}
net_dbg_ratelimited("ipv6_hop_ra: wrong RA length %d\n",
- nh[optoff + 1]);
+ tlv_hdr->tlv_len);
kfree_skb_reason(skb, SKB_DROP_REASON_IP_INHDR);
return false;
}
@@ -961,18 +962,20 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
{
- const unsigned char *nh = skb_network_header(skb);
+ int tlv_off = offsetof(struct hop_jumbo_hdr, tlv_type);
+ struct hop_jumbo_hdr *jumbo_hdr = (struct hop_jumbo_hdr *)
+ (skb_network_header(skb) + optoff - tlv_off);
SKB_DR(reason);
u32 pkt_len;
- if (nh[optoff + 1] != 4 || (optoff & 3) != 2) {
+ if (jumbo_hdr->tlv_len != 4 || (optoff & 3) != 2) {
net_dbg_ratelimited("ipv6_hop_jumbo: wrong jumbo opt length/alignment %d\n",
- nh[optoff+1]);
+ jumbo_hdr->tlv_len);
SKB_DR_SET(reason, IP_INHDR);
goto drop;
}
- pkt_len = ntohl(*(__be32 *)(nh + optoff + 2));
+ pkt_len = ntohl(jumbo_hdr->jumbo_payload_len);
if (pkt_len <= IPV6_MAXPLEN) {
icmpv6_param_prob_reason(skb, ICMPV6_HDR_FIELD, optoff + 2,
SKB_DROP_REASON_IP_INHDR);
@@ -1004,15 +1007,16 @@ static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
static bool ipv6_hop_calipso(struct sk_buff *skb, int optoff)
{
- const unsigned char *nh = skb_network_header(skb);
+ struct hop_calipso_hdr *calipso_hdr =
+ (struct hop_calipso_hdr *)(skb_network_header(skb) + optoff);
- if (nh[optoff + 1] < 8)
+ if (calipso_hdr->tlv_len < 8)
goto drop;
- if (nh[optoff + 6] * 4 + 8 > nh[optoff + 1])
+ if (calipso_hdr->cmpt_len * 4 + 8 > calipso_hdr->tlv_len)
goto drop;
- if (!calipso_validate(skb, nh + optoff))
+ if (!calipso_validate(skb, (const unsigned char *)calipso_hdr))
goto drop;
return true;
--
2.36.1
On 2/21/24 13:03, Richard Gobert wrote:
> This patch introduces 'hop_tlv_hdr' and 'hop_calipso_hdr' structs, in
> order to access fields in a readable way in "ip6_parse_tlv".
>
> Signed-off-by: Richard Gobert <[email protected]>
> ---
> include/net/ipv6.h | 16 ++++++++++++++++
> net/ipv6/exthdrs.c | 30 +++++++++++++++++-------------
> 2 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index cf25ea21d770..61677946ed46 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -151,6 +151,22 @@ struct frag_hdr {
> __be32 identification;
> };
>
> +struct hop_tlv_hdr {
> + u8 tlv_type;
> + u8 tlv_len;
> +};
> +
> +/* CALIPSO RFC 5570 */
> +struct hop_calipso_hdr {
> + u8 tlv_type;
> + u8 tlv_len;
> + u32 domain_interpretation;
> + u8 cmpt_len;
> + u8 sens_lvl;
> + u16 checksum;
> + u64 cmpt_bitmap;
> +} __packed;
> +
> /*
> * Jumbo payload option, as described in RFC 2675 2.
> */
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 4952ae792450..5db624299da4 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -114,7 +114,7 @@ static bool ip6_parse_tlv(bool hopbyhop,
> struct sk_buff *skb,
> int max_count)
> {
> - int len = (skb_transport_header(skb)[1] + 1) << 3;
> + int len = ipv6_optlen((struct ipv6_opt_hdr *)skb_transport_header(skb));
> const unsigned char *nh = skb_network_header(skb);
> int off = skb_network_header_len(skb);
> bool disallow_unknowns = false;
> @@ -890,15 +890,16 @@ static inline struct net *ipv6_skb_net(struct sk_buff *skb)
>
> static bool ipv6_hop_ra(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + struct hop_tlv_hdr *tlv_hdr =
You've forgotten to contstify your struct here and below
> + (struct hop_tlv_hdr *)(skb_network_header(skb) + optoff);
>
> - if (nh[optoff + 1] == 2) {
> + if (tlv_hdr->tlv_len == 2) {
> IP6CB(skb)->flags |= IP6SKB_ROUTERALERT;
> - memcpy(&IP6CB(skb)->ra, nh + optoff + 2, sizeof(IP6CB(skb)->ra));
> + memcpy(&IP6CB(skb)->ra, tlv_hdr + 1, sizeof(IP6CB(skb)->ra));
> return true;
> }
> net_dbg_ratelimited("ipv6_hop_ra: wrong RA length %d\n",
> - nh[optoff + 1]);
> + tlv_hdr->tlv_len);
> kfree_skb_reason(skb, SKB_DROP_REASON_IP_INHDR);
> return false;
> }
> @@ -961,18 +962,20 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
>
> static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + int tlv_off = offsetof(struct hop_jumbo_hdr, tlv_type);
> + struct hop_jumbo_hdr *jumbo_hdr = (struct hop_jumbo_hdr *)
> + (skb_network_header(skb) + optoff - tlv_off);
> SKB_DR(reason);
> u32 pkt_len;
>
> - if (nh[optoff + 1] != 4 || (optoff & 3) != 2) {
> + if (jumbo_hdr->tlv_len != 4 || (optoff & 3) != 2) {
> net_dbg_ratelimited("ipv6_hop_jumbo: wrong jumbo opt length/alignment %d\n",
> - nh[optoff+1]);
> + jumbo_hdr->tlv_len);
> SKB_DR_SET(reason, IP_INHDR);
> goto drop;
> }
>
> - pkt_len = ntohl(*(__be32 *)(nh + optoff + 2));
> + pkt_len = ntohl(jumbo_hdr->jumbo_payload_len);
> if (pkt_len <= IPV6_MAXPLEN) {
> icmpv6_param_prob_reason(skb, ICMPV6_HDR_FIELD, optoff + 2,
> SKB_DROP_REASON_IP_INHDR);
> @@ -1004,15 +1007,16 @@ static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
>
> static bool ipv6_hop_calipso(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + struct hop_calipso_hdr *calipso_hdr =
> + (struct hop_calipso_hdr *)(skb_network_header(skb) + optoff);
>
> - if (nh[optoff + 1] < 8)
> + if (calipso_hdr->tlv_len < 8)
> goto drop;
>
> - if (nh[optoff + 6] * 4 + 8 > nh[optoff + 1])
> + if (calipso_hdr->cmpt_len * 4 + 8 > calipso_hdr->tlv_len)
> goto drop;
>
> - if (!calipso_validate(skb, nh + optoff))
> + if (!calipso_validate(skb, (const unsigned char *)calipso_hdr))
> goto drop;
>
> return true;
On 2/21/24 3:03 AM, Richard Gobert wrote:
> This patch introduces 'hop_tlv_hdr' and 'hop_calipso_hdr' structs, in
> order to access fields in a readable way in "ip6_parse_tlv".
>
> Signed-off-by: Richard Gobert <[email protected]>
> ---
> include/net/ipv6.h | 16 ++++++++++++++++
> net/ipv6/exthdrs.c | 30 +++++++++++++++++-------------
> 2 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index cf25ea21d770..61677946ed46 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -151,6 +151,22 @@ struct frag_hdr {
> __be32 identification;
> };
>
> +struct hop_tlv_hdr {
> + u8 tlv_type;
> + u8 tlv_len;
> +};
> +
> +/* CALIPSO RFC 5570 */
> +struct hop_calipso_hdr {
> + u8 tlv_type;
> + u8 tlv_len;
> + u32 domain_interpretation;
> + u8 cmpt_len;
> + u8 sens_lvl;
> + u16 checksum;
> + u64 cmpt_bitmap;
> +} __packed;
> +
> /*
> * Jumbo payload option, as described in RFC 2675 2.
> */
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 4952ae792450..5db624299da4 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -114,7 +114,7 @@ static bool ip6_parse_tlv(bool hopbyhop,
> struct sk_buff *skb,
> int max_count)
> {
> - int len = (skb_transport_header(skb)[1] + 1) << 3;
> + int len = ipv6_optlen((struct ipv6_opt_hdr *)skb_transport_header(skb));
> const unsigned char *nh = skb_network_header(skb);
> int off = skb_network_header_len(skb);
> bool disallow_unknowns = false;
> @@ -890,15 +890,16 @@ static inline struct net *ipv6_skb_net(struct sk_buff *skb)
>
> static bool ipv6_hop_ra(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + struct hop_tlv_hdr *tlv_hdr =
> + (struct hop_tlv_hdr *)(skb_network_header(skb) + optoff);
>
> - if (nh[optoff + 1] == 2) {
> + if (tlv_hdr->tlv_len == 2) {
> IP6CB(skb)->flags |= IP6SKB_ROUTERALERT;
> - memcpy(&IP6CB(skb)->ra, nh + optoff + 2, sizeof(IP6CB(skb)->ra));
> + memcpy(&IP6CB(skb)->ra, tlv_hdr + 1, sizeof(IP6CB(skb)->ra));
> return true;
> }
> net_dbg_ratelimited("ipv6_hop_ra: wrong RA length %d\n",
> - nh[optoff + 1]);
> + tlv_hdr->tlv_len);
> kfree_skb_reason(skb, SKB_DROP_REASON_IP_INHDR);
> return false;
> }
> @@ -961,18 +962,20 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
>
> static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + int tlv_off = offsetof(struct hop_jumbo_hdr, tlv_type);
> + struct hop_jumbo_hdr *jumbo_hdr = (struct hop_jumbo_hdr *)
> + (skb_network_header(skb) + optoff - tlv_off);
> SKB_DR(reason);
> u32 pkt_len;
>
> - if (nh[optoff + 1] != 4 || (optoff & 3) != 2) {
> + if (jumbo_hdr->tlv_len != 4 || (optoff & 3) != 2) {
> net_dbg_ratelimited("ipv6_hop_jumbo: wrong jumbo opt length/alignment %d\n",
> - nh[optoff+1]);
> + jumbo_hdr->tlv_len);
> SKB_DR_SET(reason, IP_INHDR);
> goto drop;
> }
>
> - pkt_len = ntohl(*(__be32 *)(nh + optoff + 2));
> + pkt_len = ntohl(jumbo_hdr->jumbo_payload_len);
> if (pkt_len <= IPV6_MAXPLEN) {
> icmpv6_param_prob_reason(skb, ICMPV6_HDR_FIELD, optoff + 2,
> SKB_DROP_REASON_IP_INHDR);
> @@ -1004,15 +1007,16 @@ static bool ipv6_hop_jumbo(struct sk_buff *skb, int optoff)
>
> static bool ipv6_hop_calipso(struct sk_buff *skb, int optoff)
> {
> - const unsigned char *nh = skb_network_header(skb);
> + struct hop_calipso_hdr *calipso_hdr =
> + (struct hop_calipso_hdr *)(skb_network_header(skb) + optoff);
>
> - if (nh[optoff + 1] < 8)
> + if (calipso_hdr->tlv_len < 8)
> goto drop;
>
> - if (nh[optoff + 6] * 4 + 8 > nh[optoff + 1])
> + if (calipso_hdr->cmpt_len * 4 + 8 > calipso_hdr->tlv_len)
> goto drop;
>
> - if (!calipso_validate(skb, nh + optoff))
> + if (!calipso_validate(skb, (const unsigned char *)calipso_hdr))
> goto drop;
>
> return true;
In addition to restoring the const that Denis commented, this really
wants to be multiple patches in a set. Also, it would be best to have
selftests as well that exercise the code paths.
David Ahern wrote:
> In addition to restoring the const that Denis commented, this really
> wants to be multiple patches in a set. Also, it would be best to have
> selftests as well that exercise the code paths.
>
Thanks for the comment.
I'll think about relevant selftests, and submit a v2 with the changes.