[PATCH net 0/2] ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

[PATCH net 0/2] ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

[Olivier Matz]

The following code returns EFAULT (Bad address):

  s = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
  setsockopt(s, SOL_IPV6, IPV6_HDRINCL, 1);
  sendto(ipv6_icmp6_packet, addr);   /* returns -1, errno = EFAULT */

The problem is fixed in the second patch. The first one aligns the
code to ipv4, to avoid a race condition in the second patch.

Olivier Matz (2):
  ipv6: use READ_ONCE() for inet->hdrincl as in ipv4
  ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

 net/ipv6/raw.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH net 1/2] ipv6: use READ_ONCE() for inet->hdrincl as in ipv4

[Olivier Matz]

As it was done in commit 8f659a03a0ba ("net: ipv4: fix for a race
condition in raw_sendmsg") and commit 20b50d79974e ("net: ipv4: emulate
READ_ONCE() on ->hdrincl bit-field in raw_sendmsg()") for ipv4, copy the
value of inet->hdrincl in a local variable, to avoid introducing a race
condition in the next commit.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 net/ipv6/raw.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 96a3559f2a09..af2f9a833c4e 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -783,6 +783,7 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	struct flowi6 fl6;
 	struct ipcm6_cookie ipc6;
 	int addr_len = msg->msg_namelen;
+	int hdrincl;
 	u16 proto;
 	int err;
 
@@ -796,6 +797,13 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (msg->msg_flags & MSG_OOB)
 		return -EOPNOTSUPP;
 
+	/* hdrincl should be READ_ONCE(inet->hdrincl)
+	 * but READ_ONCE() doesn't work with bit fields.
+	 * Doing this indirectly yields the same result.
+	 */
+	hdrincl = inet->hdrincl;
+	hdrincl = READ_ONCE(hdrincl);
+
 	/*
 	 *	Get and verify the address.
 	 */
@@ -908,7 +916,7 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		fl6.flowi6_oif = np->ucast_oif;
 	security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
 
-	if (inet->hdrincl)
+	if (hdrincl)
 		fl6.flowi6_flags |= FLOWI_FLAG_KNOWN_NH;
 
 	if (ipc6.tclass < 0)
@@ -931,7 +939,7 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		goto do_confirm;
 
 back_from_confirm:
-	if (inet->hdrincl)
+	if (hdrincl)
 		err = rawv6_send_hdrinc(sk, msg, len, &fl6, &dst,
 					msg->msg_flags, &ipc6.sockc);
 	else {
-- 
2.11.0

[PATCH net 2/2] ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

[Olivier Matz]

The following code returns EFAULT (Bad address):

  s = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
  setsockopt(s, SOL_IPV6, IPV6_HDRINCL, 1);
  sendto(ipv6_icmp6_packet, addr);   /* returns -1, errno = EFAULT */

The IPv4 equivalent code works. A workaround is to use IPPROTO_RAW
instead of IPPROTO_ICMPV6.

The failure happens because 2 bytes are eaten from the msghdr by
rawv6_probe_proto_opt() starting from commit 19e3c66b52ca ("ipv6
equivalent of "ipv4: Avoid reading user iov twice after
raw_probe_proto_opt""), but at that time it was not a problem because
IPV6_HDRINCL was not yet introduced.

Only eat these 2 bytes if hdrincl == 0.

Fixes: 715f504b1189 ("ipv6: add IPV6_HDRINCL option for raw sockets")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 net/ipv6/raw.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index af2f9a833c4e..1bb88b4b677b 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -895,11 +895,14 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	opt = ipv6_fixup_options(&opt_space, opt);
 
 	fl6.flowi6_proto = proto;
-	rfv.msg = msg;
-	rfv.hlen = 0;
-	err = rawv6_probe_proto_opt(&rfv, &fl6);
-	if (err)
-		goto out;
+
+	if (!hdrincl) {
+		rfv.msg = msg;
+		rfv.hlen = 0;
+		err = rawv6_probe_proto_opt(&rfv, &fl6);
+		if (err)
+			goto out;
+	}
 
 	if (!ipv6_addr_any(daddr))
 		fl6.daddr = *daddr;
-- 
2.11.0

Re: [PATCH net 2/2] ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

[Nicolas Dichtel]

Le 06/06/2019 à 09:15, Olivier Matz a écrit :
> The following code returns EFAULT (Bad address):
> 
>   s = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
>   setsockopt(s, SOL_IPV6, IPV6_HDRINCL, 1);
>   sendto(ipv6_icmp6_packet, addr);   /* returns -1, errno = EFAULT */
> 
> The IPv4 equivalent code works. A workaround is to use IPPROTO_RAW
> instead of IPPROTO_ICMPV6.
> 
> The failure happens because 2 bytes are eaten from the msghdr by
> rawv6_probe_proto_opt() starting from commit 19e3c66b52ca ("ipv6
> equivalent of "ipv4: Avoid reading user iov twice after
> raw_probe_proto_opt""), but at that time it was not a problem because
> IPV6_HDRINCL was not yet introduced.
> 
> Only eat these 2 bytes if hdrincl == 0.
> 
> Fixes: 715f504b1189 ("ipv6: add IPV6_HDRINCL option for raw sockets")
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Re: [PATCH net 0/2] ipv6: fix EFAULT on sendto with icmpv6 and hdrincl

[David Miller]

From: Olivier Matz <olivier.matz@6wind.com>
Date: Thu,  6 Jun 2019 09:15:17 +0200

> The following code returns EFAULT (Bad address):
> 
>   s = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
>   setsockopt(s, SOL_IPV6, IPV6_HDRINCL, 1);
>   sendto(ipv6_icmp6_packet, addr);   /* returns -1, errno = EFAULT */
> 
> The problem is fixed in the second patch. The first one aligns the
> code to ipv4, to avoid a race condition in the second patch.

Series applied and queued up for -stable.

Thanks.