Commit 084ebf7c authored by Kees Cook's avatar Kees Cook
Browse files

execve: Keep bprm->argmin behind CONFIG_MMU 

When argmin was added in commit 655c16a8 ("exec: separate
MM_ANONPAGES and RLIMIT_STACK accounting"), it was intended only for
validating stack limits on CONFIG_MMU[1]. All checking for reaching the
limit (argmin) is wrapped in CONFIG_MMU ifdef checks, though setting
argmin was not. That argmin is only supposed to be used under CONFIG_MMU
was rediscovered recently[2], and I don't want to trip over this again.

Move argmin's declaration into the existing CONFIG_MMU area, and add
helpers functions so the MMU tests can be consolidated.

Link: https://lore.kernel.org/all/20181126122307.GA1660@redhat.com [1]
Link: https://lore.kernel.org/all/202406211253.7037F69@keescook/ [2]
Link: https://lore.kernel.org/r/20240621205046.4001362-1-kees@kernel.org


Signed-off-by: default avatarKees Cook <kees@kernel.org>
parent 2a97388a

fs/exec.c

+20 −6
Original line number Diff line number Diff line
@@ -486,6 +486,23 @@ static int count_strings_kernel(const char *const *argv) 
	return i;

}



static inline int bprm_set_stack_limit(struct linux_binprm *bprm,

				       unsigned long limit)

{

#ifdef CONFIG_MMU

	bprm->argmin = bprm->p - limit;

#endif

	return 0;

}

static inline bool bprm_hit_stack_limit(struct linux_binprm *bprm)

{

#ifdef CONFIG_MMU

	return bprm->p < bprm->argmin;

#else

	return false;

#endif

}



/*

 * Calculate bprm->argmin from:

 * - _STK_LIM
@@ -532,8 +549,7 @@ static int bprm_stack_limits(struct linux_binprm *bprm) 
		return -E2BIG;

	limit -= ptr_size;



	bprm->argmin = bprm->p - limit;

	return 0;

	return bprm_set_stack_limit(bprm, limit);

}



/*
@@ -571,10 +587,8 @@ static int copy_strings(int argc, struct user_arg_ptr argv, 
		pos = bprm->p;

		str += len;

		bprm->p -= len;

#ifdef CONFIG_MMU

		if (bprm->p < bprm->argmin)

		if (bprm_hit_stack_limit(bprm))

			goto out;

#endif



		while (len > 0) {

			int offset, bytes_to_copy;
@@ -649,7 +663,7 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm) 
	/* We're going to work our way backwards. */

	arg += len;

	bprm->p -= len;

	if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)

	if (bprm_hit_stack_limit(bprm))

		return -E2BIG;



	while (len > 0) {

fs/exec_test.c

+2 −0
Original line number Diff line number Diff line
@@ -96,7 +96,9 @@ static void exec_test_bprm_stack_limits(struct kunit *test) 


		rc = bprm_stack_limits(&bprm);

		KUNIT_EXPECT_EQ_MSG(test, rc, result->expected_rc, "on loop %d", i);

#ifdef CONFIG_MMU

		KUNIT_EXPECT_EQ_MSG(test, bprm.argmin, result->expected_argmin, "on loop %d", i);

#endif

	}

}

include/linux/binfmts.h

+1 −1
Original line number Diff line number Diff line
@@ -19,13 +19,13 @@ struct linux_binprm { 
#ifdef CONFIG_MMU

	struct vm_area_struct *vma;

	unsigned long vma_pages;

	unsigned long argmin; /* rlimit marker for copy_strings() */

#else

# define MAX_ARG_PAGES	32

	struct page *page[MAX_ARG_PAGES];

#endif

	struct mm_struct *mm;

	unsigned long p; /* current top of mem */

	unsigned long argmin; /* rlimit marker for copy_strings() */

	unsigned int

		/* Should an execfd be passed to userspace? */

		have_execfd:1,