Message-id: <31564068.33481385370637554.JavaMail.weblogic@epv6ml06>
Date:	Mon, 25 Nov 2013 09:10:40 +0000 (GMT)
From:	Anurag Aggarwal <a.anurag@...sung.com>
To:	Dave Martin <Dave.Martin@....com>,
	Anurag Aggarwal <anurag19aggarwal@...il.com>
Cc:	Naveen Kumar <naveen.sel@...sung.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	Narendra Meher <narendra.m1@...sung.com>,
	"nico@...aro.org" <nico@...aro.org>,
	Anurag Aggarwal <a.anurag@...sung.com>,
	Catalin Marinas <catalin.marinas@....com>,
	"will.deacon@....com" <will.deacon@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"cpgs ." <cpgs@...sung.com>,
	"naveenkrishna.ch@...il.com" <naveenkrishna.ch@...il.com>,
	Rajat Suri <rajat.suri@...sung.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Mohammad Irfan Ansari <mohammad.a2@...sung.com>
Subject: Re: Re: [PATCH] ARM: unwinder: Handle Stackoverflow in unwind_exec_insn

Hi Dave,

I think that we don't need to avoid stack overflow completely but we need to avoid data being derefrenced that is not part of stack.

I agree with you that there is no rule in ABI to guarantee that stack overflow will only occur when backtracking last set of register.

From my understanding of code the only chances of getting a data abort is while executing these four instructions:
1) 1000iiii iiiiiiii : Pop up to 12 integer registers under masks {r15-r12}, {r11-r4}
2) 10100nnn : Pop r4-r[4+nnn]
3) 10101nnn : Pop r4-r[4+nnn], r14
4) 10110001 0000iiii : Pop integer registers under mask {r3, r2, r1, r0}

I think it would be better to execute these instruction in their own seperate functions, which would be called from unwind_exec_insn, and in those function depending on the depth of stack remaining we can decide whether it is possible to execute the instruction or not.

The above method will add some extra code but will avoid additional checks that are not required every where.

and solve our purpose also as we need to avoid data abort due to stack overflow not stack overflow completely.

Regards
Anurag Aggarwal

------- Original Message -------
Sender : Dave Martin<Dave.Martin@....com>
Date : Nov 23, 2013 01:07 (GMT+05:30)
Title : Re: [PATCH] ARM: unwinder: Handle Stackoverflow in unwind_exec_insn

On Sat, Nov 09, 2013 at 12:28:57PM +0530, Anurag Aggarwal wrote:
> Thanks for your input Dave,
> 
> I think there is another way to avoid the stack overflow and reduce
> the number of checks also,
> 
> Stack overflow will cause a problem only when we are backtracking the
> last set of registers.
> i.e when the difference between current SP and top of stack is less
> than or equal to number of registers

Apologies, it looks like I failed to respond to this earlier...


Although that will usually be correct, there is no rule in the ABI to
guarantee it.

> we can create two unwind_exec_insn, one without checks and one with checks.
> 
> then we call the correct function from unwind_frame depending on the
> difference of SP and top of stack.
> 
> This will reduce the amount of checks every time we read a set of
> registers from stack

That sounds like it might duplicate a lot of code, to optimise based on
assumptions that may not always be true, for what really should not be a
hot path in the kernel.

If you can find a tidy way of doing it, it would be certainly worth
reviewing, but I still think it would be simpler just to do a simple
bounds check for every word read from the stack -- it should be
impossible for that to go wrong, even if some of the bounds checks
are not stictly required.

Cheers
---Dave

Powered by blists - more mailing lists