linux-kernel - Re: [tip:sched/urgent] sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 20 Sep 2014 16:28:00 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Chuck Ebbert <cebbert.lkml@...il.com>
Cc:	linux-kernel@...r.kernel.org, hpa@...or.com,
	james.hogan@...tec.com, atomlin@...hat.com, tglx@...utronix.de
Subject: Re: [tip:sched/urgent] sched: Fix end_of_stack()  and location of
 stack canary for architectures using CONFIG_STACK_GROWSUP


* Chuck Ebbert <cebbert.lkml@...il.com> wrote:

> On Fri, 19 Sep 2014 23:42:37 -0700
> tip-bot for Chuck Ebbert <tipbot@...or.com> wrote:
> 
> > Commit-ID:  a3215fb47c7ecb814dc16815245db4f375841268
> > Gitweb:     http://git.kernel.org/tip/a3215fb47c7ecb814dc16815245db4f375841268
> > Author:     Chuck Ebbert <cebbert.lkml@...il.com>
> > AuthorDate: Fri, 19 Sep 2014 09:35:05 -0500
> > Committer:  Ingo Molnar <mingo@...nel.org>
> > CommitDate: Sat, 20 Sep 2014 08:38:16 +0200
> > 
> > sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
> > 
> > Aaron Tomlin recently posted patches to enable checking the
> > stack canary on every task switch:
> > 
> >   http://lkml.org/lkml/2014/9/12/293
> > 
> > Looking at the canary code, I realized that every arch
> > (except ia64, which adds some space for register spill
> > above the stack) shares a definition of end_of_stack()
> > that makes it the first long after the threadinfo.
> > 
> > For stacks that grow down, this low address is correct
> > because the stack starts at the end of the thread area
> > and grows toward lower addresses. However, for stacks
> > that grow up, toward higher addresses, this is wrong.
> > (The stack actually grows away from the canary.)
> > 
> > On these archs end_of_stack() should return the address
> > of the last long, at the highest possible address for
> > the stack.
> > 
> > Signed-off-by: Chuck Ebbert <cebbert.lkml@...il.com>
> > Tested-by: James Hogan <james.hogan@...tec.com> [metag]
> > Acked-by: James Hogan <james.hogan@...tec.com>
> > Acked-by: Aaron Tomlin <atomlin@...hat.com>
> > Link: http://lkml.kernel.org/r/20140919093505.62681e43@as
> > [ Added comments to end_of_stack(). ]
> > Signed-off-by: Ingo Molnar <mingo@...nel.org>
> > ---
> >  include/linux/sched.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 5c2c885..0e20a24 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2608,9 +2608,22 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_struct
> >  	task_thread_info(p)->task = p;
> >  }
> >  
> > +/*
> > + * Return the address of the first long before (or after,
> > + * depending on the architecture's default stack growth
> > + * direction) * a task's task_info structure, which is the
> > + * kernel stack's last usable spot.
> > + *
> > + * This is the point of no return, if the stack grows
> > + * beyond that position, we corrupt the task's state.
> > + */
> 
> This comment you added is not correct for archs where the stack grows
> up. The threadinfo is always at the lowest address on the stack in
> every case. Instead of corrupting the thread info, a stack overflow
> will corrupt whatever is on the next page after the stack if it grows
> upward.

Okay, I've zapped the commit, please resubmit the patch with the 
comment corrected.

Thanks,

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