Re: [PATCH 1/1] ipc/shm: fix the historical/wrong mm->start_stack check

[Oleg Nesterov <oleg@redhat.com>]

On 08/23, Cyrill Gorcunov wrote:
>
> On Sat, Aug 23, 2014 at 04:43:27PM +0200, Oleg Nesterov wrote:
> > The ->start_stack check in do_shmat() looks ugly and simply wrong.
> >
> > 1. ->start_stack is only valid right after exec(), the application
> >    can switch to another stack and even unmap this area.
> >
> > 2. The reason for this check is not clear at all. The application
> >    should know what it does. And why 4 pages? And why in fact it
> >    requires 5 pages?
> >
> > 3. This wrongly assumes that the stack can only grown down.
> >
> > Personally I think we should simply kill this check, but I did not
> > dare to do this. So the patch only fixes the 1st problem (mostly to
> > avoid the usage of mm->start_stack) and ignores VM_GROWSUP.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Thanks!

> I don't understand this check either, the comment above it says nothing
> but only commits what code is doing not explaining why.

Yes, and this check predates the git history. I even looked into
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git but this
change was added by the huge "v2.5.0.7 -> v2.5.0.8" update in 2002,
and obviously without any explanation (apart from "fix up proper shmat
semantics", but this connects SHM_REMAP itself).

Oleg.