From: ebiederm@xmission.com (Eric W. Biederman)
To: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>,
Eric Dumazet <eric.dumazet@gmail.com>,
Kostya Serebryany <kcc@google.com>,
Andrey Konovalov <andreyknvl@google.com>,
Eric Dumazet <edumazet@google.com>,
Network Development <netdev@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
security@kernel.org, Alexander Potapenko <glider@google.com>,
linux-sctp@vger.kernel.org, Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>
Subject: [PATCH net] net/sctp: Always set scope_id in sctp_inet6_skb_msgname
Date: Thu, 16 Nov 2017 04:17:48 +0000 [thread overview]
Message-ID: <871skyzwk3.fsf_-_@xmission.com> (raw)
In-Reply-To: <CAG_fn=UUB6ep_hdiQq7CyGUnc93R-13mGZERyM-yi3b11hLRdQ@mail.gmail.com> (Alexander Potapenko's message of "Wed, 15 Nov 2017 09:22:39 +0100")
Alexandar Potapenko while testing the kernel with KMSAN and syzkaller
discovered that in some configurations sctp would leak 4 bytes of
kernel stack.
Working with his reproducer I discovered that those 4 bytes that
are leaked is the scope id of an ipv6 address returned by recvmsg.
With a little code inspection and a shrewd guess I discovered that
sctp_inet6_skb_msgname only initializes the scope_id field for link
local ipv6 addresses to the interface index the link local address
pertains to instead of initializing the scope_id field for all ipv6
addresses.
That is almost reasonable as scope_id's are meaniningful only for link
local addresses. Set the scope_id in all other cases to 0 which is
not a valid interface index to make it clear there is nothing useful
in the scope_id field.
There should be no danger of breaking userspace as the stack leak
guaranteed that previously meaningless random data was being returned.
Cc: stable@vger.kernel.org
Fixes: 372f525b495c ("SCTP: Resync with LKSCTP tree.")
History-tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Reported-by: Alexander Potapenko <glider@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
net/sctp/ipv6.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index a6dfa86c0201..3b18085e3b10 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -807,9 +807,10 @@ static void sctp_inet6_skb_msgname(struct sk_buff *skb, char *msgname,
addr->v6.sin6_flowinfo = 0;
addr->v6.sin6_port = sh->source;
addr->v6.sin6_addr = ipv6_hdr(skb)->saddr;
- if (ipv6_addr_type(&addr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL) {
+ if (ipv6_addr_type(&addr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL)
addr->v6.sin6_scope_id = sctp_v6_skb_iif(skb);
- }
+ else
+ addr->v6.sin6_scope_id = 0;
}
*addr_len = sctp_v6_addr_to_user(sctp_sk(skb->sk), addr);
--
2.14.1
WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>,
Eric Dumazet <eric.dumazet@gmail.com>,
Kostya Serebryany <kcc@google.com>,
Andrey Konovalov <andreyknvl@google.com>,
Eric Dumazet <edumazet@google.com>,
Network Development <netdev@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
security@kernel.org, Alexander Potapenko <glider@google.com>,
linux-sctp@vger.kernel.org, Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>
Subject: [PATCH net] net/sctp: Always set scope_id in sctp_inet6_skb_msgname
Date: Wed, 15 Nov 2017 22:17:48 -0600 [thread overview]
Message-ID: <871skyzwk3.fsf_-_@xmission.com> (raw)
In-Reply-To: <CAG_fn=UUB6ep_hdiQq7CyGUnc93R-13mGZERyM-yi3b11hLRdQ@mail.gmail.com> (Alexander Potapenko's message of "Wed, 15 Nov 2017 09:22:39 +0100")
Alexandar Potapenko while testing the kernel with KMSAN and syzkaller
discovered that in some configurations sctp would leak 4 bytes of
kernel stack.
Working with his reproducer I discovered that those 4 bytes that
are leaked is the scope id of an ipv6 address returned by recvmsg.
With a little code inspection and a shrewd guess I discovered that
sctp_inet6_skb_msgname only initializes the scope_id field for link
local ipv6 addresses to the interface index the link local address
pertains to instead of initializing the scope_id field for all ipv6
addresses.
That is almost reasonable as scope_id's are meaniningful only for link
local addresses. Set the scope_id in all other cases to 0 which is
not a valid interface index to make it clear there is nothing useful
in the scope_id field.
There should be no danger of breaking userspace as the stack leak
guaranteed that previously meaningless random data was being returned.
Cc: stable@vger.kernel.org
Fixes: 372f525b495c ("SCTP: Resync with LKSCTP tree.")
History-tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Reported-by: Alexander Potapenko <glider@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
net/sctp/ipv6.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index a6dfa86c0201..3b18085e3b10 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -807,9 +807,10 @@ static void sctp_inet6_skb_msgname(struct sk_buff *skb, char *msgname,
addr->v6.sin6_flowinfo = 0;
addr->v6.sin6_port = sh->source;
addr->v6.sin6_addr = ipv6_hdr(skb)->saddr;
- if (ipv6_addr_type(&addr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL) {
+ if (ipv6_addr_type(&addr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL)
addr->v6.sin6_scope_id = sctp_v6_skb_iif(skb);
- }
+ else
+ addr->v6.sin6_scope_id = 0;
}
*addr_len = sctp_v6_addr_to_user(sctp_sk(skb->sk), addr);
--
2.14.1
next prev parent reply other threads:[~2017-11-16 4:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-31 16:14 [PATCH] net: recvmsg: Unconditionally zero struct sockaddr_storage Kees Cook
2017-10-31 17:31 ` Eric Dumazet
2017-11-01 12:48 ` Eric W. Biederman
2017-11-01 18:23 ` Kees Cook
2017-11-15 8:22 ` Alexander Potapenko
2017-11-16 4:17 ` Eric W. Biederman [this message]
2017-11-16 4:17 ` [PATCH net] net/sctp: Always set scope_id in sctp_inet6_skb_msgname Eric W. Biederman
2017-11-16 14:00 ` David Miller
2017-11-16 14:00 ` David Miller
2017-11-15 2:13 ` [PATCH] net: recvmsg: Unconditionally zero struct sockaddr_storage Kees Cook
2017-11-15 18:37 ` Eric W. Biederman
2017-10-31 17:35 ` Ben Hutchings
2017-11-01 6:49 ` Willy Tarreau
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=871skyzwk3.fsf_-_@xmission.com \
--to=ebiederm@xmission.com \
--cc=andreyknvl@google.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eric.dumazet@gmail.com \
--cc=glider@google.com \
--cc=kcc@google.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=security@kernel.org \
--cc=vyasevich@gmail.com \
/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.