Skip to content

Commit

Permalink
exec: fix remove_arg_zero
Browse files Browse the repository at this point in the history
Petr Tesarik discovered a problem in remove_arg_zero(). He writes:

 When a script is loaded, load_script() replaces argv[0] with the
 name of the interpreter and the filename passed to the exec syscall.
 However, there is no guarantee that the length of the interpreter
 name plus the length of the filename is greater than the length of
 the original argv[0]. If the difference happens to cross a page boundary,
 setup_arg_pages() will call put_dirty_page() [aka install_arg_page()]
 with an address outside the VMA.

 Therefore, remove_arg_zero() must free all pages which would be unused
 after the argument is removed.

So, rewrite the remove_arg_zero function without gotos, with a few comments,
and with the commonly used explicit index/offset. This fixes the problem
and makes it easier to understand as well.

[[email protected]: add comment]
Signed-off-by: Nick Piggin <[email protected]>
Cc: Petr Tesarik <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Nick Piggin authored and Linus Torvalds committed May 8, 2007
1 parent af7c693 commit 4fc75ff
Showing 1 changed file with 34 additions and 16 deletions.
50 changes: 34 additions & 16 deletions fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -982,33 +982,51 @@ void compute_creds(struct linux_binprm *bprm)
task_unlock(current);
security_bprm_post_apply_creds(bprm);
}

EXPORT_SYMBOL(compute_creds);

/*
* Arguments are '\0' separated strings found at the location bprm->p
* points to; chop off the first by relocating brpm->p to right after
* the first '\0' encountered.
*/
void remove_arg_zero(struct linux_binprm *bprm)
{
if (bprm->argc) {
unsigned long offset;
char * kaddr;
struct page *page;
char ch;

offset = bprm->p % PAGE_SIZE;
goto inside;
do {
unsigned long offset;
unsigned long index;
char *kaddr;
struct page *page;

while (bprm->p++, *(kaddr+offset++)) {
if (offset != PAGE_SIZE)
continue;
offset = 0;
kunmap_atomic(kaddr, KM_USER0);
inside:
page = bprm->page[bprm->p/PAGE_SIZE];
offset = bprm->p & ~PAGE_MASK;
index = bprm->p >> PAGE_SHIFT;

page = bprm->page[index];
kaddr = kmap_atomic(page, KM_USER0);
}
kunmap_atomic(kaddr, KM_USER0);

/* run through page until we reach end or find NUL */
do {
ch = *(kaddr + offset);

/* discard that character... */
bprm->p++;
offset++;
} while (offset < PAGE_SIZE && ch != '\0');

kunmap_atomic(kaddr, KM_USER0);

/* free the old page */
if (offset == PAGE_SIZE) {
__free_page(page);
bprm->page[index] = NULL;
}
} while (ch != '\0');

bprm->argc--;
}
}

EXPORT_SYMBOL(remove_arg_zero);

/*
Expand Down

0 comments on commit 4fc75ff

Please sign in to comment.