[PATCH net-next 0/3] icmp6: support rfc 4884

[PATCH net-next 0/3] icmp6: support rfc 4884

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Extend the feature merged earlier this week for IPv4 to IPv6.

I expected this to be a single patch, but patch 1 seemed better to be
stand-alone

patch 1: small fix in length calculation
patch 2: factor out ipv4-specific
patch 3: add ipv6

Willem de Bruijn (3):
  icmp: revise rfc4884 tests
  icmp: prepare rfc 4884 for ipv6
  icmp6: support rfc 4884

 include/linux/icmp.h        |  3 ++-
 include/linux/ipv6.h        |  1 +
 include/uapi/linux/icmpv6.h |  1 +
 include/uapi/linux/in6.h    |  1 +
 net/ipv4/icmp.c             | 26 +++++++-------------------
 net/ipv4/ip_sockglue.c      | 14 +++++++++++++-
 net/ipv6/datagram.c         | 16 ++++++++++++++++
 net/ipv6/ipv6_sockglue.c    | 12 ++++++++++++
 8 files changed, 53 insertions(+), 21 deletions(-)

-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH net-next 1/3] icmp: revise rfc4884 tests

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

1) Only accept packets with original datagram len field >= header len.

The extension header must start after the original datagram headers.
The embedded datagram len field is compared against the 128B minimum
stipulated by RFC 4884. It is unlikely that headers extend beyond
this. But as we know the exact header length, check explicitly.

2) Remove the check that datagram length must be <= 576B.

This is a send constraint. There is no value in testing this on rx.
Within private networks it may be known safe to send larger packets.
Process these packets.

This test was also too lax. It compared original datagram length
rather than entire icmp packet length. The stand-alone fix would be:

  -       if (hlen + skb->len > 576)
  +       if (-skb_network_offset(skb) + skb->len > 576)

Fixes: eba75c587e81 ("icmp: support rfc 4884")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/ipv4/icmp.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index fd2e7a3a9eb2..646d4fb72c07 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1199,16 +1199,12 @@ void ip_icmp_error_rfc4884(const struct sk_buff *skb,
 		return;
 	}
 
-	/* outer headers up to inner iph. skb->data is at inner payload */
+	/* original datagram headers: end of icmph to payload (skb->data) */
 	hlen = -skb_transport_offset(skb) - sizeof(struct icmphdr);
 
-	/* per rfc 791: maximum packet length of 576 bytes */
-	if (hlen + skb->len > 576)
-		return;
-
 	/* per rfc 4884: minimal datagram length of 128 bytes */
 	off = icmp_hdr(skb)->un.reserved[1] * sizeof(u32);
-	if (off < 128)
+	if (off < 128 || off < hlen)
 		return;
 
 	/* kernel has stripped headers: return payload offset in bytes */
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH net-next 2/3] icmp: prepare rfc 4884 for ipv6

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

The RFC 4884 spec is largely the same between IPv4 and IPv6.
Factor out the IPv4 specific parts in preparation for IPv6 support:

- icmp types supported

- icmp header size, and thus offset to original datagram start

- datagram length field offset in icmp(6)hdr.

- datagram length field word size: 4B for IPv4, 8B for IPv6.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/icmp.h   |  3 ++-
 net/ipv4/icmp.c        | 17 ++++-------------
 net/ipv4/ip_sockglue.c | 14 +++++++++++++-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/include/linux/icmp.h b/include/linux/icmp.h
index 8fc38a34cb20..0af4d210ee31 100644
--- a/include/linux/icmp.h
+++ b/include/linux/icmp.h
@@ -37,6 +37,7 @@ static inline bool icmp_is_err(int type)
 }
 
 void ip_icmp_error_rfc4884(const struct sk_buff *skb,
-			   struct sock_ee_data_rfc4884 *out);
+			   struct sock_ee_data_rfc4884 *out,
+			   int thlen, int off);
 
 #endif	/* _LINUX_ICMP_H */
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 646d4fb72c07..1e70e98f14f8 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1186,24 +1186,15 @@ static bool ip_icmp_error_rfc4884_validate(const struct sk_buff *skb, int off)
 }
 
 void ip_icmp_error_rfc4884(const struct sk_buff *skb,
-			   struct sock_ee_data_rfc4884 *out)
+			   struct sock_ee_data_rfc4884 *out,
+			   int thlen, int off)
 {
-	int hlen, off;
-
-	switch (icmp_hdr(skb)->type) {
-	case ICMP_DEST_UNREACH:
-	case ICMP_TIME_EXCEEDED:
-	case ICMP_PARAMETERPROB:
-		break;
-	default:
-		return;
-	}
+	int hlen;
 
 	/* original datagram headers: end of icmph to payload (skb->data) */
-	hlen = -skb_transport_offset(skb) - sizeof(struct icmphdr);
+	hlen = -skb_transport_offset(skb) - thlen;
 
 	/* per rfc 4884: minimal datagram length of 128 bytes */
-	off = icmp_hdr(skb)->un.reserved[1] * sizeof(u32);
 	if (off < 128 || off < hlen)
 		return;
 
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index a5ea02d7a183..6aa45fe0a676 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -389,6 +389,18 @@ int ip_ra_control(struct sock *sk, unsigned char on,
 	return 0;
 }
 
+static void ipv4_icmp_error_rfc4884(const struct sk_buff *skb,
+				    struct sock_ee_data_rfc4884 *out)
+{
+	switch (icmp_hdr(skb)->type) {
+	case ICMP_DEST_UNREACH:
+	case ICMP_TIME_EXCEEDED:
+	case ICMP_PARAMETERPROB:
+		ip_icmp_error_rfc4884(skb, out, sizeof(struct icmphdr),
+				      icmp_hdr(skb)->un.reserved[1] * 4);
+	}
+}
+
 void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err,
 		   __be16 port, u32 info, u8 *payload)
 {
@@ -412,7 +424,7 @@ void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err,
 
 	if (skb_pull(skb, payload - skb->data)) {
 		if (inet_sk(sk)->recverr_rfc4884)
-			ip_icmp_error_rfc4884(skb, &serr->ee.ee_rfc4884);
+			ipv4_icmp_error_rfc4884(skb, &serr->ee.ee_rfc4884);
 
 		skb_reset_transport_header(skb);
 		if (sock_queue_err_skb(sk, skb) == 0)
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH net-next 3/3] icmp6: support rfc 4884

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Extend the rfc 4884 read interface introduced for ipv4 in
commit eba75c587e81 ("icmp: support rfc 4884") to ipv6.

Add socket option SOL_IPV6/IPV6_RECVERR_RFC4884.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/ipv6.h        |  1 +
 include/uapi/linux/icmpv6.h |  1 +
 include/uapi/linux/in6.h    |  1 +
 net/ipv4/icmp.c             |  1 +
 net/ipv6/datagram.c         | 16 ++++++++++++++++
 net/ipv6/ipv6_sockglue.c    | 12 ++++++++++++
 6 files changed, 32 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 8d8f877e7f81..a44789d027cc 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -283,6 +283,7 @@ struct ipv6_pinfo {
 				autoflowlabel:1,
 				autoflowlabel_set:1,
 				mc_all:1,
+				recverr_rfc4884:1,
 				rtalert_isolate:1;
 	__u8			min_hopcount;
 	__u8			tclass;
diff --git a/include/uapi/linux/icmpv6.h b/include/uapi/linux/icmpv6.h
index 2622b5a3e616..c1661febc2dc 100644
--- a/include/uapi/linux/icmpv6.h
+++ b/include/uapi/linux/icmpv6.h
@@ -68,6 +68,7 @@ struct icmp6hdr {
 #define icmp6_mtu		icmp6_dataun.un_data32[0]
 #define icmp6_unused		icmp6_dataun.un_data32[0]
 #define icmp6_maxdelay		icmp6_dataun.un_data16[0]
+#define icmp6_datagram_len	icmp6_dataun.un_data8[0]
 #define icmp6_router		icmp6_dataun.u_nd_advt.router
 #define icmp6_solicited		icmp6_dataun.u_nd_advt.solicited
 #define icmp6_override		icmp6_dataun.u_nd_advt.override
diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
index 9f2273a08356..5ad396a57eb3 100644
--- a/include/uapi/linux/in6.h
+++ b/include/uapi/linux/in6.h
@@ -179,6 +179,7 @@ struct in6_flowlabel_req {
 #define IPV6_LEAVE_ANYCAST	28
 #define IPV6_MULTICAST_ALL	29
 #define IPV6_ROUTER_ALERT_ISOLATE	30
+#define IPV6_RECVERR_RFC4884	31
 
 /* IPV6_MTU_DISCOVER values */
 #define IPV6_PMTUDISC_DONT		0
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 1e70e98f14f8..1155b6ad7a3b 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1208,6 +1208,7 @@ void ip_icmp_error_rfc4884(const struct sk_buff *skb,
 	if (!ip_icmp_error_rfc4884_validate(skb, off))
 		out->flags |= SO_EE_RFC4884_FLAG_INVALID;
 }
+EXPORT_SYMBOL_GPL(ip_icmp_error_rfc4884);
 
 int icmp_err(struct sk_buff *skb, u32 info)
 {
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 390bedde21a5..dd1d71e12b61 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -19,6 +19,7 @@
 #include <linux/route.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/icmp.h>
 
 #include <net/ipv6.h>
 #include <net/ndisc.h>
@@ -284,6 +285,17 @@ int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *uaddr,
 }
 EXPORT_SYMBOL_GPL(ip6_datagram_connect_v6_only);
 
+void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
+			     struct sock_ee_data_rfc4884 *out)
+{
+	switch (icmp6_hdr(skb)->icmp6_type) {
+	case ICMPV6_TIME_EXCEED:
+	case ICMPV6_DEST_UNREACH:
+		ip_icmp_error_rfc4884(skb, out, sizeof(struct icmp6hdr),
+				      icmp6_hdr(skb)->icmp6_datagram_len * 8);
+	}
+}
+
 void ipv6_icmp_error(struct sock *sk, struct sk_buff *skb, int err,
 		     __be16 port, u32 info, u8 *payload)
 {
@@ -313,6 +325,10 @@ void ipv6_icmp_error(struct sock *sk, struct sk_buff *skb, int err,
 	serr->port = port;
 
 	__skb_pull(skb, payload - skb->data);
+
+	if (inet6_sk(sk)->recverr_rfc4884)
+		ipv6_icmp_error_rfc4884(skb, &serr->ee.ee_rfc4884);
+
 	skb_reset_transport_header(skb);
 
 	if (sock_queue_err_skb(sk, skb))
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index add8f7912299..d4140a23974f 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -964,6 +964,14 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		np->rxopt.bits.recvfragsize = valbool;
 		retv = 0;
 		break;
+	case IPV6_RECVERR_RFC4884:
+		if (optlen < sizeof(int))
+			goto e_inval;
+		if (val < 0 || val > 1)
+			goto e_inval;
+		np->recverr_rfc4884 = valbool;
+		retv = 0;
+		break;
 	}
 
 	release_sock(sk);
@@ -1438,6 +1446,10 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		val = np->rtalert_isolate;
 		break;
 
+	case IPV6_RECVERR_RFC4884:
+		val = np->recverr_rfc4884;
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}
-- 
2.28.0.rc0.105.gf9edc3c819-goog

Re: [PATCH net-next 3/3] icmp6: support rfc 4884

[Jakub Kicinski]

On Thu, 23 Jul 2020 10:33:57 -0400 Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Extend the rfc 4884 read interface introduced for ipv4 in
> commit eba75c587e81 ("icmp: support rfc 4884") to ipv6.
> 
> Add socket option SOL_IPV6/IPV6_RECVERR_RFC4884.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

net/ipv6/datagram.c:288:6: warning: symbol 'ipv6_icmp_error_rfc4884' was not declared. Should it be static?

Re: [PATCH net-next 3/3] icmp6: support rfc 4884

[Willem de Bruijn]

On Thu, Jul 23, 2020 at 12:44 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 23 Jul 2020 10:33:57 -0400 Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Extend the rfc 4884 read interface introduced for ipv4 in
> > commit eba75c587e81 ("icmp: support rfc 4884") to ipv6.
> >
> > Add socket option SOL_IPV6/IPV6_RECVERR_RFC4884.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> net/ipv6/datagram.c:288:6: warning: symbol 'ipv6_icmp_error_rfc4884' was not declared. Should it be static?

Oops. Thanks Jakub.

I'll wait a day before respinning, in case of other feedback.

Re: [PATCH net-next 3/3] icmp6: support rfc 4884

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

Hi Willem,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Willem-de-Bruijn/icmp6-support-rfc-4884/20200723-223533
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc3b978a8971305d456b32d3f2ac13191f5a0d7
config: nios2-defconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/ipv6/datagram.c:288:6: warning: no previous prototype for 'ipv6_icmp_error_rfc4884' [-Wmissing-prototypes]
     288 | void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
         |      ^~~~~~~~~~~~~~~~~~~~~~~

vim +/ipv6_icmp_error_rfc4884 +288 net/ipv6/datagram.c

   287	
 > 288	void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
   289				     struct sock_ee_data_rfc4884 *out)
   290	{
   291		switch (icmp6_hdr(skb)->icmp6_type) {
   292		case ICMPV6_TIME_EXCEED:
   293		case ICMPV6_DEST_UNREACH:
   294			ip_icmp_error_rfc4884(skb, out, sizeof(struct icmp6hdr),
   295					      icmp6_hdr(skb)->icmp6_datagram_len * 8);
   296		}
   297	}
   298	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10817 bytes --]

Re: [PATCH net-next 3/3] icmp6: support rfc 4884

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]

Hi Willem,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Willem-de-Bruijn/icmp6-support-rfc-4884/20200723-223533
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc3b978a8971305d456b32d3f2ac13191f5a0d7
config: i386-randconfig-s001-20200723 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-93-g4c6cbe55-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> net/ipv6/datagram.c:288:6: sparse: sparse: symbol 'ipv6_icmp_error_rfc4884' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35344 bytes --]

[RFC PATCH] icmp6: ipv6_icmp_error_rfc4884() can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 datagram.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index dd1d71e12b61a5..cc8ad7ddecdaa3 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -285,8 +285,8 @@ int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *uaddr,
 }
 EXPORT_SYMBOL_GPL(ip6_datagram_connect_v6_only);
 
-void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
-			     struct sock_ee_data_rfc4884 *out)
+static void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
+				    struct sock_ee_data_rfc4884 *out)
 {
 	switch (icmp6_hdr(skb)->icmp6_type) {
 	case ICMPV6_TIME_EXCEED: