get_arg_page() && ptr_size accounting

[Oleg Nesterov <oleg@redhat.com>]

Hi Kees,

I was thinking about backporting the commit 98da7d08850fb8bde
("fs/exec.c: account for argv/envp pointers"), but I am not sure
I understand it...

So get_arg_page() does

		/*
		 * Since the stack will hold pointers to the strings, we
		 * must account for them as well.
		 *
		 * The size calculation is the entire vma while each arg page is
		 * built, so each time we get here it's calculating how far it
		 * is currently (rather than each call being just the newly
		 * added size from the arg page).  As a result, we need to
		 * always add the entire size of the pointers, so that on the
		 * last call to get_arg_page() we'll actually have the entire
		 * correct size.
		 */
		ptr_size = (bprm->argc + bprm->envc) * sizeof(void *);
		if (ptr_size > ULONG_MAX - size)
			goto fail;
		size += ptr_size;

OK, but
		acct_arg_size(bprm, size / PAGE_SIZE);

after that doesn't look exactly right. This additional space will be used later
when the process already uses bprm->mm, right? so it shouldn't be accounted by
acct_arg_size().

Not to mention that ptr_size/PAGE_SIZE doesn't look right in any case...

In short. Am I totally confused or the patch below makes sense? This way we do
not need the fat comment.

Oleg.

--- x/fs/exec.c
+++ x/fs/exec.c
@@ -222,25 +222,17 @@ static struct page *get_arg_page(struct
 		unsigned long size = bprm->vma->vm_end - bprm->vma->vm_start;
 		unsigned long ptr_size, limit;
 
+		acct_arg_size(bprm, size / PAGE_SIZE);
+
 		/*
 		 * Since the stack will hold pointers to the strings, we
 		 * must account for them as well.
-		 *
-		 * The size calculation is the entire vma while each arg page is
-		 * built, so each time we get here it's calculating how far it
-		 * is currently (rather than each call being just the newly
-		 * added size from the arg page).  As a result, we need to
-		 * always add the entire size of the pointers, so that on the
-		 * last call to get_arg_page() we'll actually have the entire
-		 * correct size.
 		 */
 		ptr_size = (bprm->argc + bprm->envc) * sizeof(void *);
 		if (ptr_size > ULONG_MAX - size)
 			goto fail;
 		size += ptr_size;
 
-		acct_arg_size(bprm, size / PAGE_SIZE);
-
 		/*
 		 * We've historically supported up to 32 pages (ARG_MAX)
 		 * of argument strings even with small stacks