Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1293325 > unrolled thread

net: heap-out-of-bounds in sock_setsockopt

Started byDmitry Vyukov <dvyukov@google.com>
First post2015-12-16 20:40 +0100
Last post2015-12-17 00:00 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  net: heap-out-of-bounds in sock_setsockopt Dmitry Vyukov <dvyukov@google.com> - 2015-12-16 20:40 +0100
    Re: net: heap-out-of-bounds in sock_setsockopt Cong Wang <cwang@twopensource.com> - 2015-12-16 21:30 +0100
      Re: net: heap-out-of-bounds in sock_setsockopt Eric Dumazet <edumazet@google.com> - 2015-12-16 22:10 +0100
      Re: net: heap-out-of-bounds in sock_setsockopt Willem de Bruijn <willemdebruijn.kernel@gmail.com> - 2015-12-17 00:00 +0100

#1293325 — net: heap-out-of-bounds in sock_setsockopt

FromDmitry Vyukov <dvyukov@google.com>
Date2015-12-16 20:40 +0100
Subjectnet: heap-out-of-bounds in sock_setsockopt
Message-ID<qGq5A-5XV-7@gated-at.bofh.it>
Hello,

The following program triggers heap-out-of-bounds access in sock_setsockopt:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <syscall.h>
#include <string.h>
#include <stdint.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <linux/in.h>
#include <linux/in6.h>

#define SOF_TIMESTAMPING_OPT_ID (1<<7)

int main()
{
        int fd = socket(PF_INET6, SOCK_RAW, IPPROTO_TCP);
        struct sockaddr_in6 sa = {};
        sa.sin6_family = AF_INET6;
        sa.sin6_port = htons(13277);
        inet_pton(AF_INET6, "::1", &sa.sin6_addr);
        connect(fd, (struct sockaddr*)&sa, sizeof(sa));
        int opt = SOF_TIMESTAMPING_OPT_ID;
        setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, &opt, sizeof(opt));
        return 0;
}


BUG: KASAN: slab-out-of-bounds in sock_setsockopt+0x1284/0x13d0 at
addr ffff88006563ec10
Read of size 4 by task syzkaller_execu/4755
=============================================================================
BUG RAWv6 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------
INFO: Allocated in sk_prot_alloc+0x69/0x340 age=17 cpu=3 pid=4755
[<      none      >] kmem_cache_alloc+0x244/0x2c0 mm/slub.c:2607
[<      none      >] sk_prot_alloc+0x69/0x340 net/core/sock.c:1343
[<      none      >] sk_alloc+0x3a/0x6b0 net/core/sock.c:1418
[<      none      >] inet6_create+0x2c4/0xfd0 net/ipv6/af_inet6.c:170
[<      none      >] __sock_create+0x37c/0x640 net/socket.c:1162
[<     inline     >] sock_create net/socket.c:1202
[<     inline     >] SYSC_socket net/socket.c:1232
[<      none      >] SyS_socket+0xef/0x1b0 net/socket.c:1212
[<      none      >] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

Call Trace:
 [<ffffffff816cec2e>] __asan_report_load4_noabort+0x3e/0x40
mm/kasan/report.c:294
 [<ffffffff84affb14>] sock_setsockopt+0x1284/0x13d0 net/core/sock.c:880
 [<     inline     >] SYSC_setsockopt net/socket.c:1746
 [<ffffffff84aed7ee>] SyS_setsockopt+0x1fe/0x240 net/socket.c:1729
 [<ffffffff85c18c76>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185


On commit edb42dc7bc0da0125ceacab810a553ce1f0cac8d (Dec 15).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1293338

FromCong Wang <cwang@twopensource.com>
Date2015-12-16 21:30 +0100
Message-ID<qGqRY-6vM-7@gated-at.bofh.it>
In reply to#1293325
On Wed, Dec 16, 2015 at 11:34 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> BUG: KASAN: slab-out-of-bounds in sock_setsockopt+0x1284/0x13d0 at
> addr ffff88006563ec10
> Read of size 4 by task syzkaller_execu/4755
> =============================================================================
> BUG RAWv6 (Not tainted): kasan: bad access detected
> -----------------------------------------------------------------------------
> INFO: Allocated in sk_prot_alloc+0x69/0x340 age=17 cpu=3 pid=4755
> [<      none      >] kmem_cache_alloc+0x244/0x2c0 mm/slub.c:2607
> [<      none      >] sk_prot_alloc+0x69/0x340 net/core/sock.c:1343
> [<      none      >] sk_alloc+0x3a/0x6b0 net/core/sock.c:1418
> [<      none      >] inet6_create+0x2c4/0xfd0 net/ipv6/af_inet6.c:170
> [<      none      >] __sock_create+0x37c/0x640 net/socket.c:1162
> [<     inline     >] sock_create net/socket.c:1202
> [<     inline     >] SYSC_socket net/socket.c:1232
> [<      none      >] SyS_socket+0xef/0x1b0 net/socket.c:1212
> [<      none      >] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
>
> Call Trace:
>  [<ffffffff816cec2e>] __asan_report_load4_noabort+0x3e/0x40
> mm/kasan/report.c:294
>  [<ffffffff84affb14>] sock_setsockopt+0x1284/0x13d0 net/core/sock.c:880
>  [<     inline     >] SYSC_setsockopt net/socket.c:1746
>  [<ffffffff84aed7ee>] SyS_setsockopt+0x1fe/0x240 net/socket.c:1729
>  [<ffffffff85c18c76>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185

Hmm, we should exclude the raw socket case, something like the
following, but I am not sure if the check is too strict or not, also
not sure if we should return an error for this raw socket case.

diff --git a/net/core/sock.c b/net/core/sock.c
index 765be83..c26e80a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -872,7 +872,7 @@ int sock_setsockopt(struct socket *sock, int
level, int optname,

                if (val & SOF_TIMESTAMPING_OPT_ID &&
                    !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {
-                       if (sk->sk_protocol == IPPROTO_TCP) {
+                       if (sk->sk_protocol == IPPROTO_TCP &&
sk->sk_type == SOCK_STREAM) {
                                if (sk->sk_state != TCP_ESTABLISHED) {
                                        ret = -EINVAL;
                                        break;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293346

FromEric Dumazet <edumazet@google.com>
Date2015-12-16 22:10 +0100
Message-ID<qGruG-70F-11@gated-at.bofh.it>
In reply to#1293338
On Wed, Dec 16, 2015 at 12:22 PM, Cong Wang <cwang@twopensource.com> wrote:

> Hmm, we should exclude the raw socket case, something like the
> following, but I am not sure if the check is too strict or not, also
> not sure if we should return an error for this raw socket case.
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 765be83..c26e80a 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -872,7 +872,7 @@ int sock_setsockopt(struct socket *sock, int
> level, int optname,
>
>                 if (val & SOF_TIMESTAMPING_OPT_ID &&
>                     !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {
> -                       if (sk->sk_protocol == IPPROTO_TCP) {
> +                       if (sk->sk_protocol == IPPROTO_TCP &&
> sk->sk_type == SOCK_STREAM) {
>                                 if (sk->sk_state != TCP_ESTABLISHED) {
>                                         ret = -EINVAL;
>                                         break;

This looks right, please post this officially ;)

tcp_sk(sk) only works for TCP sockets , and the test must include
sk->sk_type == SOCK_STREAM
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293379

FromWillem de Bruijn <willemdebruijn.kernel@gmail.com>
Date2015-12-17 00:00 +0100
Message-ID<qGtd8-7St-5@gated-at.bofh.it>
In reply to#1293338
>
> Hmm, we should exclude the raw socket case, something like the
> following, but I am not sure if the check is too strict or not, also
> not sure if we should return an error for this raw socket case.

No, SOF_TIMESTAMPING_OPT_ID with SOCK_RAW/IPPROTO_TCP
is legitimate. It should fall through to initializing sk->sk_tskey to zero.
Only stream sockets should use the special case where numbering
is bytestream and computed by subtracting the seqno from the seqno
at the time that the option is enabled.

>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 765be83..c26e80a 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -872,7 +872,7 @@ int sock_setsockopt(struct socket *sock, int
> level, int optname,
>
>                 if (val & SOF_TIMESTAMPING_OPT_ID &&
>                     !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {
> -                       if (sk->sk_protocol == IPPROTO_TCP) {
> +                       if (sk->sk_protocol == IPPROTO_TCP &&
> sk->sk_type == SOCK_STREAM) {
>                                 if (sk->sk_state != TCP_ESTABLISHED) {
>                                         ret = -EINVAL;
>                                         break;

I made the same error when later returning the tskey in
__skb_tstamp_tx:

        if (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID) {
                serr->ee.ee_data = skb_shinfo(skb)->tskey;
                if (sk->sk_protocol == IPPROTO_TCP)
                        serr->ee.ee_data -= sk->sk_tskey;
        }

This has no effect if sk->sk_tskey is initialized to 0 with your patch. Still,
it should not treat SOCK_RAW as a TCP sock here, either. Please
add this if you're about to send the patch. Or I can send it separately,
if you prefer. Thanks for the quick fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web