diff mbox series

net: ipv4: fix for a race condition in raw_sendmsg

Message ID 5a2caf2e.4ce61c0a.5017a.575f@mx.google.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series net: ipv4: fix for a race condition in raw_sendmsg | expand

Commit Message

simo.ghannam@gmail.com Dec. 10, 2017, 3:50 a.m. UTC 
From: Mohamed Ghannam <simo.ghannam@gmail.com>

inet->hdrincl is racy, and could lead to uninitialized stack pointer
usage, so its value should be read only once.

Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
---
 net/ipv4/raw.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Eric Dumazet Dec. 10, 2017, 4:30 a.m. UTC | #1 
On Sun, 2017-12-10 at 03:50 +0000, simo.ghannam@gmail.com wrote:
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
> 
> inet->hdrincl is racy, and could lead to uninitialized stack pointer
> usage, so its value should be read only once.
> 
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
> ---
>  net/ipv4/raw.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Thanks a lot for fixing this very serious bug.

Reviewed-by: Eric Dumazet <edumazet@google.com>

Please David add :

Fixes: c008ba5bdc9f ("ipv4: Avoid reading user iov twice after raw_probe_proto_opt")

Thanks !
David Miller Dec. 11, 2017, 7:05 p.m. UTC | #2 
From: simo.ghannam@gmail.com
Date: Sun, 10 Dec 2017 03:50:58 +0000

> From: Mohamed Ghannam <simo.ghannam@gmail.com>
> 
> inet->hdrincl is racy, and could lead to uninitialized stack pointer
> usage, so its value should be read only once.
> 
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>

Applied and queued up for -stable with Fixes: tag from Eric Dumazet added.

Thanks.
diff mbox series

Patch

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 33b70bfd1122..125c1eab3eaa 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -513,11 +513,16 @@  static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int err;
 	struct ip_options_data opt_copy;
 	struct raw_frag_vec rfv;
+	int hdrincl;
 
 	err = -EMSGSIZE;
 	if (len > 0xFFFF)
 		goto out;
 
+	/* hdrincl should be READ_ONCE(inet->hdrincl)
+	 * but READ_ONCE() doesn't work with bit fields
+	 */
+	hdrincl = inet->hdrincl;
 	/*
 	 *	Check the flags.
 	 */
@@ -593,7 +598,7 @@  static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		/* Linux does not mangle headers on raw sockets,
 		 * so that IP options + IP_HDRINCL is non-sense.
 		 */
-		if (inet->hdrincl)
+		if (hdrincl)
 			goto done;
 		if (ipc.opt->opt.srr) {
 			if (!daddr)
@@ -615,12 +620,12 @@  static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
 			   RT_SCOPE_UNIVERSE,
-			   inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
+			   hdrincl ? IPPROTO_RAW : sk->sk_protocol,
 			   inet_sk_flowi_flags(sk) |
-			    (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
+			    (hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
 			   daddr, saddr, 0, 0, sk->sk_uid);
 
-	if (!inet->hdrincl) {
+	if (!hdrincl) {
 		rfv.msg = msg;
 		rfv.hlen = 0;
 
@@ -645,7 +650,7 @@  static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		goto do_confirm;
 back_from_confirm:
 
-	if (inet->hdrincl)
+	if (hdrincl)
 		err = raw_send_hdrinc(sk, &fl4, msg, len,
 				      &rt, msg->msg_flags, &ipc.sockc);