Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 18 Dec 2020 00:28:03 +0300
From:   Cyrill Gorcunov <gorcunov@gmail.com>
To:     Keno Fischer <keno@juliacomputing.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>, mkoutny@suse.com,
        ktkhai@virtuozzo.com, Andrey Vagin <avagin@gmail.com>,
        Dmitry Safonov <0x7f454c46@gmail.com>, CriuML <criu@openvz.org>
Subject: Re: brk checks in PR_SET_MM code
Message-ID: <20201217212803.GE14556@grain>
References: <CABV8kRwoHAAdez8k60O+AJ9E3g5_PM0F6tpbpB9dC115_FD3Eg@mail.gmail.com>
 <20201217074202.GD14556@grain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201217074202.GD14556@grain>
User-Agent: Mutt/1.14.6 (2020-07-11)
Precedence: bulk

On Thu, Dec 17, 2020 at 10:42:02AM +0300, Cyrill Gorcunov wrote:
> On Wed, Dec 16, 2020 at 08:29:30PM -0500, Keno Fischer wrote:
> > Hi all,
> > 
> > The code in prctl(PR_SET_MM, ...) performs a number of sanity checks,
> > among them
> > 
> > ```
> > /*
> >  * @brk should be after @end_data in traditional maps.
> >  */
> > if (prctl_map->start_brk <= prctl_map->end_data ||
> >     prctl_map->brk <= prctl_map->end_data)
> > goto out;
> > ```
> > 
> 
> Thanks for pointing, Keno! I don't remember the details right now,
> gimme some time and once I refresh my memory I'll reply with
> details.

Indeed, when loaded via ld directly we've got a different layout:

# /lib64/ld-linux-x86-64.so.2 ~/t

start_code	7fc25b0a4000
end_code	7fc25b0c4524
start_stack	7fffcc6b2400
start_data	7fc25b0ce4c0
end_data	7fc25b0cff98
start_brk	55555710c000
sbrk(0)		55555710c000

Note though that as far as I understand the layout is provided by
ld loader. I contrast the regular load

# ~/t

start_code	401000
end_code	401a15
start_stack	7ffce4577dd0
start_data	403e10
end_data	40408c
start_brk	b5b000
sbrk(0)		b5b000

I fear we've not been using ld's loaded programs in c/r procedure much
that's why it has not been noted earlier. Need to think how to fix it.
Using the whole memory map for verification procedure is a correct way
thus the commit you mention is doing exactly what it should but we need
to figure out how to deal with fdpic loaded files... I'll back once I
figure it out (hopefully more-less soon). Thanks a huge for report!