Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 24 Jun 2017 11:15:04 -0400
From: Brad Spengler <spender@...ecurity.net>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: oss-security@...ts.openwall.com, pageexec@...email.hu
Subject: Re: More CONFIG_VMAP_STACK vulnerabilities, refcount_t UAF, and an
 ignored Secure Boot bypass / rootkit method

On Fri, Jun 23, 2017 at 06:04:00PM -0700, Linus Torvalds wrote:
> On Fri, Jun 23, 2017 at 5:50 PM, Brad Spengler <spender@...ecurity.net> wrote:
> >
> > BTW, we're happy to go toe-to-toe with you here in public on actual facts
> > instead of pathetic ad hominems.
> 

Linus,

Are you backing down from providing any actual facts from your claims? (Wouldn't
be the first time) Let me provide some facts then.

https://grsecurity.net/~spender/stack_gap_fix.txt
(and since posting just the URL isn't enough):
Pulled in the 4.11 upstream fixes for the stack gap problem:

Unmerged paths:
  (use "git add <file>..." to mark resolution)

	both modified:   arch/arm/mm/mmap.c
	both modified:   arch/frv/mm/elf-fdpic.c
	both modified:   arch/mips/mm/mmap.c
	both modified:   arch/powerpc/mm/slice.c
	both modified:   arch/sh/mm/mmap.c
	both modified:   arch/sparc/kernel/sys_sparc_64.c
	both modified:   arch/sparc/mm/hugetlbpage.c
	both modified:   arch/x86/kernel/sys_x86_64.c
	both modified:   arch/x86/mm/hugetlbpage.c
	both modified:   fs/hugetlbfs/inode.c
	both modified:   mm/mmap.c

Huh, this sure is a whole lot of merge conflicts, generally only
happens when someone's copy+pasting code and renaming things.
Let's take a look at some of these rejects (HEAD is PaX, the ugly vm_start_gap
is the upstream fix):

fs/hugetlbfs/inode.c:
<<<<<<< HEAD
                if (TASK_SIZE - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (TASK_SIZE - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/arm/mm/mmap.c:
<<<<<<< HEAD
                if (TASK_SIZE - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (TASK_SIZE - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/sh/mm/mmap.c:
<<<<<<< HEAD
                if (TASK_SIZE - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (TASK_SIZE - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/x86/kernel/sys_x86_64.c:
<<<<<<< HEAD
                if (end - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (end - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/powerpc/mm/slice.c:
<<<<<<< HEAD
        return check_heap_stack_gap(vma, addr, len);
=======
        return (!vma || (addr + len) <= vm_start_gap(vma));
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/frv/mm/elf-fdpic.c:
<<<<<<< HEAD
                if (TASK_SIZE - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (TASK_SIZE - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

arch/mips/mm/mmap.c:
<<<<<<< HEAD
                if (TASK_SIZE - len >= addr && check_heap_stack_gap(vma, addr, len))
=======
                if (TASK_SIZE - len >= addr &&
                    (!vma || addr + len <= vm_start_gap(vma)))
>>>>>>> 9f3069116ed29013d0eb89e02def9effb038f850

(the rest are all the same kinds of rejects)

Let's look at our check_heap_stack_gap():

bool check_heap_stack_gap(const struct vm_area_struct *vma, unsigned long addr, unsigned long len)
{
        if (!vma) {
#ifdef CONFIG_STACK_GROWSUP
                if (addr > sysctl_heap_stack_gap)
                        vma = find_vma(current->mm, addr - sysctl_heap_stack_gap);
                else
                        vma = find_vma(current->mm, 0);
                if (vma && (vma->vm_flags & VM_GROWSUP))
                        return false;
#endif
                return true;
        }

        if (addr + len > vma->vm_start)
                return false;

        if (vma->vm_flags & VM_GROWSDOWN)
                return sysctl_heap_stack_gap <= vma->vm_start - addr - len;
#ifdef CONFIG_STACK_GROWSUP
        else if (vma->vm_prev && (vma->vm_prev->vm_flags & VM_GROWSUP))
                return addr - vma->vm_prev->vm_end >= sysctl_heap_stack_gap;
#endif

        return true;
}

Amazing, it took them 4 whole weeks just to rip off PaX's fix from 2010.

So Linus, you called the patches garbage when someone asked how we fixed the heap
stack gap issue 7 years ago when you failed to.  Can you provide any technical details
demonstrating why that fix is garbage, the fix that looks very similar in form and
function to what's present upstream now finally (in some of the kernels at least, and
minus ours being configurable and cleaner)?
Can you explain how our fix breaks userland and how your 2010 fix didn't?

If not, I'd suggest you keep your lies and FUD to yourself.  No one but lackeys for
your cult of personality are buying it.  You and others trot out the same old tired
excuse about "breaking userland" and never offer up any real facts.  If it were the
case, it wouldn't be possible to run grsec on anything but distros with recompiled
userland, and yet we work just fine on any distro.  It's a meaningless crutch for
people apparently have never looked at any kernel code of ours who refuse to accept
the simple facts:
1) You're not security experts
2) You view security as an annoyance
3) The Linux kernel's security track record is terrible

It's the only way they can justify it in their minds -- the problem surely can't
be that you've ignored the problem for years and lied to people telling them it's
the best that can be done.  When some outside group proves you wrong, you have to
pretend there's no way you could have done what they did, because you care so much
about code quality.  How can you explain the verbatim copy+pasting of our code if
that's the case?  Please explain to the world how if our code is such garbage, you
haven't been able to come up with any significant security improvements without it?

Put up or shut up, for once.

> Please.
Please.

-Brad

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.