LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Keno Fischer <keno@juliacomputing.com>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	Andrey Vagin <avagin@gmail.com>,
	Kirill Tkhai <ktkhai@virtuozzo.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH] prctl: allow to setup brk for et_dyn executables
Date: Wed, 21 Jul 2021 01:25:41 +0300	[thread overview]
Message-ID: <YPdNZfoYtIhkmbkf@grain> (raw)
In-Reply-To: <20210720145148.cdcfa3210e39e88a591af12b@linux-foundation.org>

On Tue, Jul 20, 2021 at 02:51:48PM -0700, Andrew Morton wrote:
> > 
> > This of course prevent criu from restoring such programs.
> > Looking into how kernel operates with brk/start_brk inside
> > brk() syscall I don't see any problem if we allow to setup
> > brk/start_brk without checking for end_data. Even if someone
> > pass some weird address here on a purpose then the worst
> > possible result will be an unexpected unmapping of existing
> > vma (own vma, since prctl works with the callers memory) but
> > test for RLIMIT_DATA is still valid and a user won't be able
> > to gain more memory in case of expanding VMAs via new values
> > shipped with prctl call.
> 
> So...  do you recall why you added that test originally?

To be honest, when I added this test in first place I simply forgot
about et_dyn executables because we usually run executables via
traditional exec call (where brk map sits before end_data VMA),
not via loader and that's the reason why I didn't hit this problem
before and why this get revealed only after a couple of years.
This is simply rarely used.

> 
> This is under prctl(CAP_SET_MM), yes?  What capabilities does this
> require?

Yes, it is for prctl(PR_SET_MM_MAP) and requires no additional
caps. The most important thing here is check_data_rlimit() function
which called at the end of memory map verification -- we make sure
the user won't get more memory than been granted by RLIMIT_DATA limit
even if he passes some bad brk value here on a purpose.

	/*
	 * Neither we should allow to override limits if they set.
	 */
	if (check_data_rlimit(rlimit(RLIMIT_DATA), prctl_map->brk,
			      prctl_map->start_brk, prctl_map->end_data,
			      prctl_map->start_data))
			goto out;

which expands to (I wrapped code to make it a bit more readable)

static inline int check_data_rlimit(unsigned long rlim,
				    unsigned long new,
				    unsigned long start,
				    unsigned long end_data,
				    unsigned long start_data)
{
	if (rlimit(RLIMIT_DATA) < RLIM_INFINITY) {
		if (((prctl_map->brk - prctl_map->start_brk) +
		     (prctl_map->end_data - prctl_map->start_data)) > rlimit(RLIMIT_DATA))
			return -ENOSPC;
	}

	return 0;
}

      reply	other threads:[~2021-07-20 22:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21 22:12 Cyrill Gorcunov
2021-01-22  9:17 ` Kirill Tkhai
2021-07-20  7:33 ` Andrei Vagin
2021-07-20 13:54   ` Cyrill Gorcunov
2021-07-20 21:51 ` Andrew Morton
2021-07-20 22:25   ` Cyrill Gorcunov [this message]

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=YPdNZfoYtIhkmbkf@grain \
    --to=gorcunov@gmail.com \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=keno@juliacomputing.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH] prctl: allow to setup brk for et_dyn executables' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).