| Message ID | 1309773814-2669-5-git-send-email-paolo.pisati@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 04.07.2011 12:03, paolo.pisati@canonical.com wrote: > From: Oleg Nesterov <oleg@redhat.com> > > BugLink: http://bugs.launchpad.net/bugs/804234 > > commit 114279be2120a916e8a04feeb2ac976a10016f2f upstream. > > Note: this patch targets 2.6.37 and tries to be as simple as possible. > That is why it adds more copy-and-paste horror into fs/compat.c and > uglifies fs/exec.c, this will be cleanuped later. > > compat_copy_strings() plays with bprm->vma/mm directly and thus has > two problems: it lacks the RLIMIT_STACK check and argv/envp memory > is not visible to oom killer. > > Export acct_arg_size() and get_arg_page(), change compat_copy_strings() > to use get_arg_page(), change compat_do_execve() to do acct_arg_size(0) > as do_execve() does. > > Add the fatal_signal_pending/cond_resched checks into compat_count() and > compat_copy_strings(), this matches the code in fs/exec.c and certainly > makes sense. > > CVE-2010-NNN3 > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Andi Kleen <ak@linux.intel.com> > Cc: Moritz Muehlenhoff <jmm@debian.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> > --- > fs/compat.c | 28 +++++++++++++++------------- > fs/exec.c | 8 ++++---- > include/linux/binfmts.h | 4 ++++ > 3 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/fs/compat.c b/fs/compat.c > index 6d6f98f..fc6eeae 100644 > --- a/fs/compat.c > +++ b/fs/compat.c > @@ -1360,6 +1360,10 @@ static int compat_count(compat_uptr_t __user *argv, int max) > argv++; > if (i++ >= max) > return -E2BIG; > + > + if (fatal_signal_pending(current)) > + return -ERESTARTNOHAND; > + cond_resched(); > } > } > return i; > @@ -1401,6 +1405,12 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, > while (len > 0) { > int offset, bytes_to_copy; > > + if (fatal_signal_pending(current)) { > + ret = -ERESTARTNOHAND; > + goto out; > + } > + cond_resched(); > + > offset = pos % PAGE_SIZE; > if (offset == 0) > offset = PAGE_SIZE; > @@ -1417,18 +1427,8 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, > if (!kmapped_page || kpos != (pos & PAGE_MASK)) { > struct page *page; > > -#ifdef CONFIG_STACK_GROWSUP > - ret = expand_stack_downwards(bprm->vma, pos); > - if (ret < 0) { > - /* We've exceed the stack rlimit. */ > - ret = -E2BIG; > - goto out; > - } > -#endif > - ret = get_user_pages(current, bprm->mm, pos, > - 1, 1, 1, &page, NULL); > - if (ret <= 0) { > - /* We've exceed the stack rlimit. */ > + page = get_arg_page(bprm, pos, 1); > + if (!page) { > ret = -E2BIG; > goto out; > } > @@ -1549,8 +1549,10 @@ int compat_do_execve(char * filename, > return retval; > > out: > - if (bprm->mm) > + if (bprm->mm) { > + acct_arg_size(bprm, 0); > mmput(bprm->mm); > + } > > out_file: > if (bprm->file) { > diff --git a/fs/exec.c b/fs/exec.c > index 7b125a8..3bc6c91 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -159,7 +159,7 @@ out: > > #ifdef CONFIG_MMU > > -static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) > +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) > { > struct mm_struct *mm = current->mm; > long diff = (long)(pages - bprm->vma_pages); > @@ -174,7 +174,7 @@ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) > up_write(&mm->mmap_sem); > } > > -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > int write) > { > struct page *page; > @@ -291,11 +291,11 @@ static bool valid_arg_len(struct linux_binprm *bprm, long len) > > #else > > -static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) > +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) > { > } > > -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > int write) > { > struct page *page; > diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h > index 48564ac..7f6c8e8 100644 > --- a/include/linux/binfmts.h > +++ b/include/linux/binfmts.h > @@ -60,6 +60,10 @@ struct linux_binprm{ > unsigned long loader, exec; > }; > > +extern void acct_arg_size(struct linux_binprm *bprm, unsigned long pages); > +extern struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, > + int write); > + > #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0 > #define BINPRM_FLAGS_ENFORCE_NONDUMP (1 << BINPRM_FLAGS_ENFORCE_NONDUMP_BIT) > I don't find CVE-2010-NNN3 somehow. But NNN3 is usually only temporary and then replaced by a real number...
On 04.07.2011 17:35, Stefan Bader wrote: > On 04.07.2011 12:03, paolo.pisati@canonical.com wrote: >> From: Oleg Nesterov <oleg@redhat.com> >> >> BugLink: http://bugs.launchpad.net/bugs/804234 >> >> commit 114279be2120a916e8a04feeb2ac976a10016f2f upstream. >> >> Note: this patch targets 2.6.37 and tries to be as simple as possible. >> That is why it adds more copy-and-paste horror into fs/compat.c and >> uglifies fs/exec.c, this will be cleanuped later. >> >> compat_copy_strings() plays with bprm->vma/mm directly and thus has >> two problems: it lacks the RLIMIT_STACK check and argv/envp memory >> is not visible to oom killer. >> >> Export acct_arg_size() and get_arg_page(), change compat_copy_strings() >> to use get_arg_page(), change compat_do_execve() to do acct_arg_size(0) >> as do_execve() does. >> >> Add the fatal_signal_pending/cond_resched checks into compat_count() and >> compat_copy_strings(), this matches the code in fs/exec.c and certainly >> makes sense. >> >> CVE-2010-NNN3 >> >> Signed-off-by: Oleg Nesterov <oleg@redhat.com> >> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> >> Signed-off-by: Andi Kleen <ak@linux.intel.com> >> Cc: Moritz Muehlenhoff <jmm@debian.org> >> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> >> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> >> --- >> fs/compat.c | 28 +++++++++++++++------------- >> fs/exec.c | 8 ++++---- >> include/linux/binfmts.h | 4 ++++ >> 3 files changed, 23 insertions(+), 17 deletions(-) >> >> diff --git a/fs/compat.c b/fs/compat.c >> index 6d6f98f..fc6eeae 100644 >> --- a/fs/compat.c >> +++ b/fs/compat.c >> @@ -1360,6 +1360,10 @@ static int compat_count(compat_uptr_t __user *argv, int max) >> argv++; >> if (i++ >= max) >> return -E2BIG; >> + >> + if (fatal_signal_pending(current)) >> + return -ERESTARTNOHAND; >> + cond_resched(); >> } >> } >> return i; >> @@ -1401,6 +1405,12 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, >> while (len > 0) { >> int offset, bytes_to_copy; >> >> + if (fatal_signal_pending(current)) { >> + ret = -ERESTARTNOHAND; >> + goto out; >> + } >> + cond_resched(); >> + >> offset = pos % PAGE_SIZE; >> if (offset == 0) >> offset = PAGE_SIZE; >> @@ -1417,18 +1427,8 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, >> if (!kmapped_page || kpos != (pos & PAGE_MASK)) { >> struct page *page; >> >> -#ifdef CONFIG_STACK_GROWSUP >> - ret = expand_stack_downwards(bprm->vma, pos); >> - if (ret < 0) { >> - /* We've exceed the stack rlimit. */ >> - ret = -E2BIG; >> - goto out; >> - } >> -#endif >> - ret = get_user_pages(current, bprm->mm, pos, >> - 1, 1, 1, &page, NULL); >> - if (ret <= 0) { >> - /* We've exceed the stack rlimit. */ >> + page = get_arg_page(bprm, pos, 1); >> + if (!page) { >> ret = -E2BIG; >> goto out; >> } >> @@ -1549,8 +1549,10 @@ int compat_do_execve(char * filename, >> return retval; >> >> out: >> - if (bprm->mm) >> + if (bprm->mm) { >> + acct_arg_size(bprm, 0); >> mmput(bprm->mm); >> + } >> >> out_file: >> if (bprm->file) { >> diff --git a/fs/exec.c b/fs/exec.c >> index 7b125a8..3bc6c91 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -159,7 +159,7 @@ out: >> >> #ifdef CONFIG_MMU >> >> -static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) >> +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) >> { >> struct mm_struct *mm = current->mm; >> long diff = (long)(pages - bprm->vma_pages); >> @@ -174,7 +174,7 @@ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) >> up_write(&mm->mmap_sem); >> } >> >> -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, >> +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, >> int write) >> { >> struct page *page; >> @@ -291,11 +291,11 @@ static bool valid_arg_len(struct linux_binprm *bprm, long len) >> >> #else >> >> -static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) >> +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) >> { >> } >> >> -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, >> +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, >> int write) >> { >> struct page *page; >> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h >> index 48564ac..7f6c8e8 100644 >> --- a/include/linux/binfmts.h >> +++ b/include/linux/binfmts.h >> @@ -60,6 +60,10 @@ struct linux_binprm{ >> unsigned long loader, exec; >> }; >> >> +extern void acct_arg_size(struct linux_binprm *bprm, unsigned long pages); >> +extern struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, >> + int write); >> + >> #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0 >> #define BINPRM_FLAGS_ENFORCE_NONDUMP (1 << BINPRM_FLAGS_ENFORCE_NONDUMP_BIT) >> > > I don't find CVE-2010-NNN3 somehow. But NNN3 is usually only temporary and then > replaced by a real number... > Think I found it: the full number for that seems to be CVE-2010-4243. If that looks ok to you, I would replace the CVE number in the patch. -Stefan
On Mon, Jul 04, 2011 at 05:50:43PM +0200, Stefan Bader wrote: > > I don't find CVE-2010-NNN3 somehow. But NNN3 is usually only temporary and then > > replaced by a real number... > > > Think I found it: the full number for that seems to be CVE-2010-4243. If that > looks ok to you, I would replace the CVE number in the patch. Right NNN3 and 4243 were merged at the end of last week. There are now two commits required to fix the latter. -apw
diff --git a/fs/compat.c b/fs/compat.c index 6d6f98f..fc6eeae 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1360,6 +1360,10 @@ static int compat_count(compat_uptr_t __user *argv, int max) argv++; if (i++ >= max) return -E2BIG; + + if (fatal_signal_pending(current)) + return -ERESTARTNOHAND; + cond_resched(); } } return i; @@ -1401,6 +1405,12 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, while (len > 0) { int offset, bytes_to_copy; + if (fatal_signal_pending(current)) { + ret = -ERESTARTNOHAND; + goto out; + } + cond_resched(); + offset = pos % PAGE_SIZE; if (offset == 0) offset = PAGE_SIZE; @@ -1417,18 +1427,8 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, if (!kmapped_page || kpos != (pos & PAGE_MASK)) { struct page *page; -#ifdef CONFIG_STACK_GROWSUP - ret = expand_stack_downwards(bprm->vma, pos); - if (ret < 0) { - /* We've exceed the stack rlimit. */ - ret = -E2BIG; - goto out; - } -#endif - ret = get_user_pages(current, bprm->mm, pos, - 1, 1, 1, &page, NULL); - if (ret <= 0) { - /* We've exceed the stack rlimit. */ + page = get_arg_page(bprm, pos, 1); + if (!page) { ret = -E2BIG; goto out; } @@ -1549,8 +1549,10 @@ int compat_do_execve(char * filename, return retval; out: - if (bprm->mm) + if (bprm->mm) { + acct_arg_size(bprm, 0); mmput(bprm->mm); + } out_file: if (bprm->file) { diff --git a/fs/exec.c b/fs/exec.c index 7b125a8..3bc6c91 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -159,7 +159,7 @@ out: #ifdef CONFIG_MMU -static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) { struct mm_struct *mm = current->mm; long diff = (long)(pages - bprm->vma_pages); @@ -174,7 +174,7 @@ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) up_write(&mm->mmap_sem); } -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, int write) { struct page *page; @@ -291,11 +291,11 @@ static bool valid_arg_len(struct linux_binprm *bprm, long len) #else -static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) +void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) { } -static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, +struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, int write) { struct page *page; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 48564ac..7f6c8e8 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -60,6 +60,10 @@ struct linux_binprm{ unsigned long loader, exec; }; +extern void acct_arg_size(struct linux_binprm *bprm, unsigned long pages); +extern struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, + int write); + #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0 #define BINPRM_FLAGS_ENFORCE_NONDUMP (1 << BINPRM_FLAGS_ENFORCE_NONDUMP_BIT)