Skip to content

Commit

Permalink
exec: Avoid pathological argc, envc, and bprm->p values
Browse files Browse the repository at this point in the history
Make sure nothing goes wrong with the string counters or the bprm's
belief about the stack pointer. Add checks and matching self-tests.

Take special care for !CONFIG_MMU, since argmin is not exposed there.

For 32-bit validation, 32-bit UML was used:
$ tools/testing/kunit/kunit.py run \
	--make_options CROSS_COMPILE=i686-linux-gnu- \
	--make_options SUBARCH=i386 \
	exec

For !MMU validation, m68k was used:
$ tools/testing/kunit/kunit.py run \
	--arch m68k --make_option CROSS_COMPILE=m68k-linux-gnu- \
	exec

Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
  • Loading branch information
kees committed Jul 14, 2024
1 parent 084ebf7 commit 21f9310
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
10 changes: 9 additions & 1 deletion fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,9 @@ static inline int bprm_set_stack_limit(struct linux_binprm *bprm,
unsigned long limit)
{
#ifdef CONFIG_MMU
/* Avoid a pathological bprm->p. */
if (bprm->p < limit)
return -E2BIG;
bprm->argmin = bprm->p - limit;
#endif
return 0;
Expand Down Expand Up @@ -531,6 +534,9 @@ static int bprm_stack_limits(struct linux_binprm *bprm)
* of argument strings even with small stacks
*/
limit = max_t(unsigned long, limit, ARG_MAX);
/* Reject totally pathological counts. */
if (bprm->argc < 0 || bprm->envc < 0)
return -E2BIG;
/*
* We must account for the size of all the argv and envp pointers to
* the argv and envp strings, since they will also take up space in
Expand All @@ -544,7 +550,9 @@ static int bprm_stack_limits(struct linux_binprm *bprm)
* argc can never be 0, to keep them from walking envp by accident.
* See do_execveat_common().
*/
ptr_size = (max(bprm->argc, 1) + bprm->envc) * sizeof(void *);
if (check_add_overflow(max(bprm->argc, 1), bprm->envc, &ptr_size) ||
check_mul_overflow(ptr_size, sizeof(void *), &ptr_size))
return -E2BIG;
if (limit <= ptr_size)
return -E2BIG;
limit -= ptr_size;
Expand Down
28 changes: 27 additions & 1 deletion fs/exec_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,34 @@ struct bprm_stack_limits_result {
};

static const struct bprm_stack_limits_result bprm_stack_limits_results[] = {
/* Giant values produce -E2BIG */
/* Negative argc/envc counts produce -E2BIG */
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = INT_MIN, .envc = INT_MIN }, .expected_rc = -E2BIG },
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = 5, .envc = -1 }, .expected_rc = -E2BIG },
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = -1, .envc = 10 }, .expected_rc = -E2BIG },
/* The max value of argc or envc is MAX_ARG_STRINGS. */
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = INT_MAX, .envc = INT_MAX }, .expected_rc = -E2BIG },
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = MAX_ARG_STRINGS, .envc = MAX_ARG_STRINGS }, .expected_rc = -E2BIG },
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = 0, .envc = MAX_ARG_STRINGS }, .expected_rc = -E2BIG },
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = MAX_ARG_STRINGS, .envc = 0 }, .expected_rc = -E2BIG },
/*
* On 32-bit system these argc and envc counts, while likely impossible
* to represent within the associated TASK_SIZE, could overflow the
* limit calculation, and bypass the ptr_size <= limit check.
*/
{ { .p = ULONG_MAX, .rlim_stack.rlim_cur = ULONG_MAX,
.argc = 0x20000001, .envc = 0x20000001 }, .expected_rc = -E2BIG },
#ifdef CONFIG_MMU
/* Make sure a pathological bprm->p doesn't cause an overflow. */
{ { .p = sizeof(void *), .rlim_stack.rlim_cur = ULONG_MAX,
.argc = 10, .envc = 10 }, .expected_rc = -E2BIG },
#endif
/*
* 0 rlim_stack will get raised to ARG_MAX. With 1 string pointer,
* we should see p - ARG_MAX + sizeof(void *).
Expand Down Expand Up @@ -88,6 +113,7 @@ static void exec_test_bprm_stack_limits(struct kunit *test)
/* Double-check the constants. */
KUNIT_EXPECT_EQ(test, _STK_LIM, SZ_8M);
KUNIT_EXPECT_EQ(test, ARG_MAX, 32 * SZ_4K);
KUNIT_EXPECT_EQ(test, MAX_ARG_STRINGS, 0x7FFFFFFF);

for (int i = 0; i < ARRAY_SIZE(bprm_stack_limits_results); i++) {
const struct bprm_stack_limits_result *result = &bprm_stack_limits_results[i];
Expand Down

0 comments on commit 21f9310

Please sign in to comment.