All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: <linux-mips@linux-mips.org>, <Zubair.Kakakhel@imgtec.com>,
	<geert+renesas@glider.be>, <david.daney@cavium.com>,
	<peterz@infradead.org>, <paul.gortmaker@windriver.com>,
	<davidlohr@hp.com>, <macro@linux-mips.org>, <chenhc@lemote.com>,
	<richard@nod.at>, <zajec5@gmail.com>, <james.hogan@imgtec.com>,
	<keescook@chromium.org>, <alex@alex-smith.me.uk>,
	<tglx@linutronix.de>, <blogic@openwrt.org>,
	<jchandra@broadcom.com>, <paul.burton@imgtec.com>,
	<qais.yousef@imgtec.com>, <linux-kernel@vger.kernel.org>,
	<ralf@linux-mips.org>, <markos.chandras@imgtec.com>,
	<dengcheng.zhu@imgtec.com>, <manuel.lauss@gmail.com>,
	<akpm@linux-foundation.org>, <lars.persson@axis.com>
Subject: Re: [PATCH v2 0/3] MIPS executable stack protection
Date: Thu, 9 Oct 2014 15:18:56 -0700	[thread overview]
Message-ID: <543709D0.6000501@imgtec.com> (raw)
In-Reply-To: <5437015B.3010205@gmail.com>

On 10/09/2014 02:42 PM, David Daney wrote:
> On 10/09/2014 01:00 PM, Leonid Yegoshin wrote:
>> The following series implements an executable stack protection in MIPS.
>>
>> It sets up a per-thread 'VDSO' page and appropriate TLB support.
>> Page is set write-protected from user and is maintained via kernel VA.
>> MIPS FPU emulation is shifted to new page and stack is relieved for
>> execute protection as is as all data pages in default setup during ELF
>> binary initialization. The real protection is controlled by GLIBC and
>> it can do stack protected now as it is done in other architectures and
>> I learned today that GLIBC team is ready for this.
>
> What does it mean to be 'ready'?  If they committed patches before 
> there was kernel support, that it putting the cart before the horse.  
> GlibC's state cannot be used as valid reason for committing major 
> kernel changes.  There would be no regression in any GLibC based 
> system as a result of not merging this patch.
Rich Fuhler said me that they discussed it internally and have a 
solution to fix their problem (ignoring PT_GNU_STACK on first library 
load - they need to sort out the logic). But we need to split both issue 
- right now stack can't be protected because of emulation. If they set 
stack protected then emulation fails on CPU without FPU.

>
>>
>> Note: actual execute-protection depends from HW capability, of course.
>>
>> This patch is required for MIPS32/64 R2 emulation on MIPS R6 
>> architecture.
>> Without it 'ssh-keygen' crashes pretty fast on attempt to execute 
>> instruction
>> in stack.
>
> There is much more blocking MIPS32/64 R2 emulation on MIPS R6 than 
> just this patch isn't there?

This one is critical - ssh-keygen crashes during running MIPS R2. I have 
a patch in my R6 repository but GLIBC still can't set stack executable 
and security suffers.

>
> Also, if you are supporting MIPS R6, this patch doesn't even work, 
> because it doesn't handle PC relative instructions at all.

It seems like you missed my statement - adding support for PC-relative 
instruction is just 5 lines of code. I just refrain from this until 
toolchain starts generating that.

Besides that, this version 2 of patch just passed 20-22 hours on P5600 
and Virtuoso (no FPU on both) under SOAK test and it gets around 1 per 
hour of signal right at emulated instruction in VDSO and unwind works 
(as I can see in debug prints).

>
>
> The recent discussions on this subject, including many comments from 
> Imgtec e-mail addresses, brought to light the need to use an 
> instruction set emulator for newer MIPSr6 ISA processors.

In Imgtec I am only one who works on MIPS R6 SW and FPU branch emulation 
and I say you - it is not needed, this solution is enough.

>
> In light of this, why does it make sense to merge this patch, instead 
> of taking the approach of emulating the instructions in the delay slot?

Well, because it does exist now. But full MIPS emulator... for all 
ASEs... for any MIPS vendor... I even doesn't want to estimate an amount 
of time and code size to develop it.

Besides that, you missed my another statement - we don't force customer 
to disclose all details of their COP2 instructions.

- Leonid

WARNING: multiple messages have this Message-ID (diff)
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: linux-mips@linux-mips.org, Zubair.Kakakhel@imgtec.com,
	geert+renesas@glider.be, david.daney@cavium.com,
	peterz@infradead.org, paul.gortmaker@windriver.com,
	davidlohr@hp.com, macro@linux-mips.org, chenhc@lemote.com,
	richard@nod.at, zajec5@gmail.com, james.hogan@imgtec.com,
	keescook@chromium.org, alex@alex-smith.me.uk, tglx@linutronix.de,
	blogic@openwrt.org, jchandra@broadcom.com,
	paul.burton@imgtec.com, qais.yousef@imgtec.com,
	linux-kernel@vger.kernel.org, ralf@linux-mips.org,
	markos.chandras@imgtec.com, dengcheng.zhu@imgtec.com,
	manuel.lauss@gmail.com, akpm@linux-foundation.org,
	lars.persson@axis.com
Subject: Re: [PATCH v2 0/3] MIPS executable stack protection
Date: Thu, 9 Oct 2014 15:18:56 -0700	[thread overview]
Message-ID: <543709D0.6000501@imgtec.com> (raw)
Message-ID: <20141009221856.9y_54Kgozv44SAJnTn3r0cdV6MnKZiMWNziG7Y6pyHw@z> (raw)
In-Reply-To: <5437015B.3010205@gmail.com>

On 10/09/2014 02:42 PM, David Daney wrote:
> On 10/09/2014 01:00 PM, Leonid Yegoshin wrote:
>> The following series implements an executable stack protection in MIPS.
>>
>> It sets up a per-thread 'VDSO' page and appropriate TLB support.
>> Page is set write-protected from user and is maintained via kernel VA.
>> MIPS FPU emulation is shifted to new page and stack is relieved for
>> execute protection as is as all data pages in default setup during ELF
>> binary initialization. The real protection is controlled by GLIBC and
>> it can do stack protected now as it is done in other architectures and
>> I learned today that GLIBC team is ready for this.
>
> What does it mean to be 'ready'?  If they committed patches before 
> there was kernel support, that it putting the cart before the horse.  
> GlibC's state cannot be used as valid reason for committing major 
> kernel changes.  There would be no regression in any GLibC based 
> system as a result of not merging this patch.
Rich Fuhler said me that they discussed it internally and have a 
solution to fix their problem (ignoring PT_GNU_STACK on first library 
load - they need to sort out the logic). But we need to split both issue 
- right now stack can't be protected because of emulation. If they set 
stack protected then emulation fails on CPU without FPU.

>
>>
>> Note: actual execute-protection depends from HW capability, of course.
>>
>> This patch is required for MIPS32/64 R2 emulation on MIPS R6 
>> architecture.
>> Without it 'ssh-keygen' crashes pretty fast on attempt to execute 
>> instruction
>> in stack.
>
> There is much more blocking MIPS32/64 R2 emulation on MIPS R6 than 
> just this patch isn't there?

This one is critical - ssh-keygen crashes during running MIPS R2. I have 
a patch in my R6 repository but GLIBC still can't set stack executable 
and security suffers.

>
> Also, if you are supporting MIPS R6, this patch doesn't even work, 
> because it doesn't handle PC relative instructions at all.

It seems like you missed my statement - adding support for PC-relative 
instruction is just 5 lines of code. I just refrain from this until 
toolchain starts generating that.

Besides that, this version 2 of patch just passed 20-22 hours on P5600 
and Virtuoso (no FPU on both) under SOAK test and it gets around 1 per 
hour of signal right at emulated instruction in VDSO and unwind works 
(as I can see in debug prints).

>
>
> The recent discussions on this subject, including many comments from 
> Imgtec e-mail addresses, brought to light the need to use an 
> instruction set emulator for newer MIPSr6 ISA processors.

In Imgtec I am only one who works on MIPS R6 SW and FPU branch emulation 
and I say you - it is not needed, this solution is enough.

>
> In light of this, why does it make sense to merge this patch, instead 
> of taking the approach of emulating the instructions in the delay slot?

Well, because it does exist now. But full MIPS emulator... for all 
ASEs... for any MIPS vendor... I even doesn't want to estimate an amount 
of time and code size to develop it.

Besides that, you missed my another statement - we don't force customer 
to disclose all details of their COP2 instructions.

- Leonid

  reply	other threads:[~2014-10-09 22:19 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-09 20:00 [PATCH v2 0/3] MIPS executable stack protection Leonid Yegoshin
2014-10-09 20:00 ` Leonid Yegoshin
2014-10-09 20:00 ` [PATCH v2 1/3] MIPS: mips_flush_cache_range is added Leonid Yegoshin
2014-10-09 20:00   ` Leonid Yegoshin
2014-10-09 20:00 ` [PATCH v2 2/3] MIPS: Setup an instruction emulation in VDSO protected page instead of user stack Leonid Yegoshin
2014-10-09 20:00   ` Leonid Yegoshin
2014-10-09 22:43   ` James Hogan
2014-10-09 22:43     ` James Hogan
2014-10-09 23:10     ` Leonid Yegoshin
2014-10-09 23:10       ` Leonid Yegoshin
2014-10-09 23:40       ` James Hogan
2014-10-09 23:40         ` James Hogan
2014-10-10  0:07         ` Leonid Yegoshin
2014-10-10  0:07           ` Leonid Yegoshin
2014-10-10 10:03           ` James Hogan
2014-10-10 10:03             ` James Hogan
2014-10-10 10:24             ` Peter Zijlstra
2014-10-10 22:47             ` Leonid Yegoshin
2014-10-10 22:47               ` Leonid Yegoshin
2014-10-10 22:56               ` David Daney
2014-10-10 23:40                 ` Leonid Yegoshin
2014-10-10 23:40                   ` Leonid Yegoshin
2014-10-09 20:00 ` [PATCH v2 3/3] MIPS: set stack/data protection as non-executable Leonid Yegoshin
2014-10-09 20:00   ` Leonid Yegoshin
2014-10-09 21:42 ` [PATCH v2 0/3] MIPS executable stack protection David Daney
2014-10-09 22:18   ` Leonid Yegoshin [this message]
2014-10-09 22:18     ` Leonid Yegoshin
2014-10-09 22:28     ` Paul Burton
2014-10-09 22:28       ` Paul Burton
2014-10-09 22:59     ` David Daney
2014-10-09 22:59       ` David Daney
2014-10-09 23:48       ` Leonid Yegoshin
2014-10-09 23:48         ` Leonid Yegoshin

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=543709D0.6000501@imgtec.com \
    --to=leonid.yegoshin@imgtec.com \
    --cc=Zubair.Kakakhel@imgtec.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex@alex-smith.me.uk \
    --cc=blogic@openwrt.org \
    --cc=chenhc@lemote.com \
    --cc=david.daney@cavium.com \
    --cc=davidlohr@hp.com \
    --cc=ddaney.cavm@gmail.com \
    --cc=dengcheng.zhu@imgtec.com \
    --cc=geert+renesas@glider.be \
    --cc=james.hogan@imgtec.com \
    --cc=jchandra@broadcom.com \
    --cc=keescook@chromium.org \
    --cc=lars.persson@axis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@linux-mips.org \
    --cc=manuel.lauss@gmail.com \
    --cc=markos.chandras@imgtec.com \
    --cc=paul.burton@imgtec.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=peterz@infradead.org \
    --cc=qais.yousef@imgtec.com \
    --cc=ralf@linux-mips.org \
    --cc=richard@nod.at \
    --cc=tglx@linutronix.de \
    --cc=zajec5@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.