memory leak in do_seccomp

3 messages, 2 authors, 2021-08-01 · open the first message on its own page

memory leak in do_seccomp

From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date: 2021-07-31 19:21:12

Hi All,

We had been running syzkaller on v5.10.y and a "memory leak in
do_seccomp" was being reported on it. I got some time to check that
today and have managed to get a syzkaller
reproducer. I dont have a C reproducer which I can share but I can use
the syz-reproducer to reproduce this with next-20210730.
The old report on v5.10.y is at
https://elisa-builder-00.iol.unh.edu/syzkaller/report?id=f6ddd3b592f00e95f9cbd2e74f70a5b04b015c6f

BUG: memory leak
unreferenced object 0xffff888019282c00 (size 512):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.841s)
  hex dump (first 32 bytes):
    01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000762c0963>] do_seccomp+0x2d5/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffffc900006b5000 (size 4096):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.841s)
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000854901e5>] __vmalloc_node_range+0x550/0x9a0
    [<000000002686628f>] __vmalloc_node+0xb5/0x100
    [<0000000004cbd298>] bpf_prog_alloc_no_stats+0x38/0x350
    [<0000000009149728>] bpf_prog_alloc+0x24/0x170
    [<000000000fe7f1e7>] bpf_prog_create_from_user+0xad/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff888026eb1000 (size 2048):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<0000000072de7240>] bpf_prog_alloc_no_stats+0xeb/0x350
    [<0000000009149728>] bpf_prog_alloc+0x24/0x170
    [<000000000fe7f1e7>] bpf_prog_create_from_user+0xad/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff888014dddac0 (size 16):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 16 bytes):
    01 00 ca 08 80 88 ff ff c8 ef df 14 80 88 ff ff  ................
  backtrace:
    [<00000000c5d4ed93>] bpf_prog_store_orig_filter+0x7b/0x1e0
    [<000000007cb21c2a>] bpf_prog_create_from_user+0x1c6/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff888014dfefc8 (size 8):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 8 bytes):
    06 00 00 00 ff ff ff 7f                          ........
  backtrace:
    [<00000000ee5550f8>] kmemdup+0x23/0x50
    [<00000000f1acd067>] bpf_prog_store_orig_filter+0x103/0x1e0
    [<000000007cb21c2a>] bpf_prog_create_from_user+0x1c6/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

Not sure if this has been already reported or not, but I will be happy
to test if you have a fix for this.


-- 
Regards
Sudip

Re: memory leak in do_seccomp

From: Kees Cook <hidden>
Date: 2021-08-01 03:30:47

On Sat, Jul 31, 2021 at 08:20:29PM +0100, Sudip Mukherjee wrote:
Hi All,

We had been running syzkaller on v5.10.y and a "memory leak in
do_seccomp" was being reported on it. I got some time to check that
today and have managed to get a syzkaller
reproducer. I dont have a C reproducer which I can share but I can use
the syz-reproducer to reproduce this with next-20210730.
The old report on v5.10.y is at
https://elisa-builder-00.iol.unh.edu/syzkaller/report?id=f6ddd3b592f00e95f9cbd2e74f70a5b04b015c6f
Thanks for the details!

Is this the same as what syzbot saw here (with a C reproducer)?
https://syzkaller.appspot.com/bug?id=2809bb0ac77ad9aa3f4afe42d6a610aba594a987

I can't figure out what happened with the "Patch testing request" that
was made; there's no link?
BUG: memory leak
unreferenced object 0xffff888019282c00 (size 512):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.841s)
  hex dump (first 32 bytes):
    01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000762c0963>] do_seccomp+0x2d5/0x27d0
Can you run "./scripts/faddr2line do_seccomp+0x2d5/0x27d0" for this? I
expect it'll be:
        sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae
The "size 512" in your v5.10.y report is from seccomp_prepare_filter()
(noted above). seccomp_prepare_filter() cleans up its error paths.
BUG: memory leak
unreferenced object 0xffffc900006b5000 (size 4096):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.841s)
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000854901e5>] __vmalloc_node_range+0x550/0x9a0
    [<000000002686628f>] __vmalloc_node+0xb5/0x100
    [<0000000004cbd298>] bpf_prog_alloc_no_stats+0x38/0x350
    [<0000000009149728>] bpf_prog_alloc+0x24/0x170
    [<000000000fe7f1e7>] bpf_prog_create_from_user+0xad/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae
Again, I'm curious about where do_seccomp+0x325/0x27d0 is for this, but
the matching one in v5.10 shows:
        ret = bpf_prog_create_from_user(&sfilter->prog, fprog,
                                        seccomp_check_filter, save_orig);

This and everything remaining below else has bpf_prog_create_from_user()
in the allocation path.
BUG: memory leak
unreferenced object 0xffff888026eb1000 (size 2048):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<0000000072de7240>] bpf_prog_alloc_no_stats+0xeb/0x350
    [<0000000009149728>] bpf_prog_alloc+0x24/0x170
    [<000000000fe7f1e7>] bpf_prog_create_from_user+0xad/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff888014dddac0 (size 16):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 16 bytes):
    01 00 ca 08 80 88 ff ff c8 ef df 14 80 88 ff ff  ................
These are two kernel pointers:
	0xffff888008ca0001 (unaligned by 1 byte?!)
	0xffff888014dfefc8

Ah, no, this is from:

struct sock_fprog_kern {
        u16                     len;
        struct sock_filter      *filter;
};

The "ca 08 80 88 ff ff" bytes are uninitialized padding. ;) "len" has
a value of 1 (which matches the syzkaller reproducer args below of a
single BPF instruction).

        fp->orig_prog = kmalloc(sizeof(*fkprog), GFP_KERNEL);
        if (!fp->orig_prog)
                return -ENOMEM;

        fkprog = fp->orig_prog;
        fkprog->len = fprog->len;
	...
        fkprog->filter = kmemdup(fp->insns, fsize,
                                 GFP_KERNEL | __GFP_NOWARN);
  backtrace:
    [<00000000c5d4ed93>] bpf_prog_store_orig_filter+0x7b/0x1e0
    [<000000007cb21c2a>] bpf_prog_create_from_user+0x1c6/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff888014dfefc8 (size 8):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.842s)
  hex dump (first 8 bytes):
    06 00 00 00 ff ff ff 7f                          ........
This contains a userspace (likely stack) pointer, and is referenced
by the second pointer above. (i.e. kmemdup() above, but how have the
contents become a user stack pointer?)
  backtrace:
    [<00000000ee5550f8>] kmemdup+0x23/0x50
    [<00000000f1acd067>] bpf_prog_store_orig_filter+0x103/0x1e0
    [<000000007cb21c2a>] bpf_prog_create_from_user+0x1c6/0x2e0
    [<000000000c70eb02>] do_seccomp+0x325/0x27d0
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae

Not sure if this has been already reported or not, but I will be happy
to test if you have a fix for this.
I was suspecting a missing error path free near bpf_prepare_filter()
as called by bpf_prog_create_from_user() here:

        /* bpf_prepare_filter() already takes care of freeing
         * memory in case something goes wrong.
         */
        fp = bpf_prepare_filter(fp, trans);
        if (IS_ERR(fp))
                return PTR_ERR(fp);

Since only seccomp and af_packet use bpf_prog_create_from_user(),
and af_packet sets neither a "trans" callback nor save_orig. But if
"trans" fails (due to some BPF instructions seccomp doesn't support),
I'd expect this leak to be detected more often.

bpf_prepare_filter() is documented as cleaning up allocations on failure,
though I notice its cleanup differs from bpf_prog_create_from_user()'s,
which uses __bpf_prog_free() instead of __bfp_prog_release(). But
that should only make a difference for orig_prog getting freed,
and bpf_prog_store_orig_filter() should already be freeing that on
failures too.

Similarly, bpf_migrate_filter() cleanups up on failure too, so this
doesn't seem to be it:

        if (!fp->jited)
                fp = bpf_migrate_filter(fp);

        return fp;

So, I'm going to assume the missing free is somehow related to
process management, since I see the Syzkaller reproducer mentions
SECCOMP_SET_MODE_FILTER_LISTENER, fork(), and ptrace(). :)

Quoting from the v5.10.y report:
# {Threaded:true Collide:true Repeat:true RepeatTimes:0 Procs:8 Slowdown:1 Sandbox:none Fault:false FaultCall:-1 FaultNth:0 Leak:true NetInjection:true NetDevices:true NetReset:true Cgroups:true BinfmtMisc:true CloseFDs:true KCSAN:false DevlinkPCI:false USB:true VhciInjection:false Wifi:false IEEE802154:false Sysctl:true UseTmpDir:true HandleSegv:true Repro:false Trace:false}
seccomp$SECCOMP_SET_MODE_FILTER_LISTENER(0x1, 0x0, &(0x7f0000000000)={0x1, &(0x7f0000000040)=[{0x6, 0x0, 0x0, 0x7fffffff}]})
0x1 is SECCOMP_SET_MODE_FILTER
0x0 is empty flags
{0x6, 0x0, 0x0, 0x7fffffff} is
	BPF_STMT(BPF_RET, SECCOMP_RET_ALLOW | 0xffff)

For "SECCOMP_SET_MODE_FILTER_LISTENER", defined here:
https://github.com/google/syzkaller/blob/master/sys/linux/seccomp.txt#L15
I was expecting flags to include SECCOMP_FILTER_FLAG_NEW_LISTENER:
	seccomp$SECCOMP_SET_MODE_FILTER_LISTENER(
			op const[SECCOMP_SET_MODE_FILTER],
			flags flags[seccomp_flags_listener],
			arg ptr[in, sock_fprog]) fd_seccomp (breaks_returns)

For the flags:

	seccomp_flags_listener = SECCOMP_FILTER_FLAG_NEW_LISTENER,
				 SECCOMP_FILTER_FLAG_LOG_LISTENER,
				 SECCOMP_FILTER_FLAG_SPEC_ALLOW_LISTENER

which is:

	SECCOMP_FILTER_FLAG_LOG_LISTENER = 10
	SECCOMP_FILTER_FLAG_NEW_LISTENER = 8
	SECCOMP_FILTER_FLAG_SPEC_ALLOW = 4
	SECCOMP_FILTER_FLAG_SPEC_ALLOW_LISTENER = 12

How is flags 0 above? (Maybe I don't understand the syzkaller reproducer
meaning fully?)
r0 = fork()
ptrace(0x10, r0)
0x10 is PTRACE_ATTACH

My best guess is there is some LISTENER refcount state we can get into
where all the processes die, but a reference is left alive.

-Kees

-- 
Kees Cook

Re: memory leak in do_seccomp

From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date: 2021-08-01 21:11:07

Hi Kees,

On Sun, Aug 1, 2021 at 4:26 AM Kees Cook [off-list ref] wrote:
On Sat, Jul 31, 2021 at 08:20:29PM +0100, Sudip Mukherjee wrote:
quoted
Hi All,

We had been running syzkaller on v5.10.y and a "memory leak in
do_seccomp" was being reported on it. I got some time to check that
today and have managed to get a syzkaller
reproducer. I dont have a C reproducer which I can share but I can use
the syz-reproducer to reproduce this with next-20210730.
The old report on v5.10.y is at
https://elisa-builder-00.iol.unh.edu/syzkaller/report?id=f6ddd3b592f00e95f9cbd2e74f70a5b04b015c6f
Thanks for the details!

Is this the same as what syzbot saw here (with a C reproducer)?
https://syzkaller.appspot.com/bug?id=2809bb0ac77ad9aa3f4afe42d6a610aba594a987
Looks similar but it says its fixed and I still get it with the
reproducer I have.
I can't figure out what happened with the "Patch testing request" that
was made; there's no link?
Looks like it has been merged with a566a9012acd ("seccomp: don't leak
memory when filter install races")
quoted
BUG: memory leak
unreferenced object 0xffff888019282c00 (size 512):
  comm "syz-executor.1", pid 7389, jiffies 4294761829 (age 17.841s)
  hex dump (first 32 bytes):
    01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000762c0963>] do_seccomp+0x2d5/0x27d0
Can you run "./scripts/faddr2line do_seccomp+0x2d5/0x27d0" for this? I
expect it'll be:
        sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
Yes, it is from "(inlined by) seccomp_prepare_filter at kernel/seccomp.c:661".
I did:
$ scripts/faddr2line vmlinux do_seccomp+0x2d5/0x27d0
do_seccomp+0x2d5/0x27d0:
kmalloc at include/linux/slab.h:591
(inlined by) kzalloc at include/linux/slab.h:721
(inlined by) seccomp_prepare_filter at kernel/seccomp.c:661
(inlined by) seccomp_prepare_user_filter at kernel/seccomp.c:703
(inlined by) seccomp_set_mode_filter at kernel/seccomp.c:1852
(inlined by) do_seccomp at kernel/seccomp.c:1972
quoted
    [<0000000006e512d1>] do_syscall_64+0x3b/0x90
    [<0000000094ae9ff8>] entry_SYSCALL_64_after_hwframe+0x44/0xae
<snip>
My best guess is there is some LISTENER refcount state we can get into
where all the processes die, but a reference is left alive.
Will be happy to run any debug patch if you need.


-- 
Regards
Sudip
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help