Re: get_arg_page() && ptr_size accounting

2018-09-12 Thread Kees Cook
On Wed, Sep 12, 2018 at 5:27 AM, Oleg Nesterov wrote: > On 09/11, Kees Cook wrote: >> >> Oh, I like this patch! This is much cleaner. > > it's pity. cause this means I will have to actually test this change and > (worse) write the changelog ;) Hehe. I know this pain well! :) >> > @@ -410,11

Re: get_arg_page() && ptr_size accounting

2018-09-12 Thread Oleg Nesterov
On 09/12, Oleg Nesterov wrote: > > it's pity. cause this means I will have to actually test this change and > (worse) write the changelog ;) Seems to work, see v2 below, I tried to address your comments. The new helper is prepare_arg_pages(), matches setup_arg_pages/shift_arg_pages.

Re: get_arg_page() && ptr_size accounting

2018-09-12 Thread Oleg Nesterov
On 09/11, Kees Cook wrote: > > Oh, I like this patch! This is much cleaner. it's pity. cause this means I will have to actually test this change and (worse) write the changelog ;) > > @@ -410,11 +365,6 @@ static int bprm_mm_init(struct linux_binprm *bprm) > > if (!mm) > >

Re: get_arg_page() && ptr_size accounting

2018-09-11 Thread Kees Cook
Oh, I like this patch! This is much cleaner. Notes below... On Tue, Sep 11, 2018 at 7:13 AM, Oleg Nesterov wrote: > diff --git a/fs/exec.c b/fs/exec.c > index 1ebf6e5..7804a5c 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -218,55 +218,10 @@ static struct page *get_arg_page(struct linux_binprm

Re: get_arg_page() && ptr_size accounting

2018-09-11 Thread Oleg Nesterov
On 09/10, Kees Cook wrote: > > On Mon, Sep 10, 2018 at 10:43 AM, Oleg Nesterov wrote: > > > > with this patch > > > > #define MAX_ARG_STRINGS 0x7FFF > > > > doesn't match the reality. perhaps something like below makes sense just > > to make it clear, but this is cosmetic. > > Part of

Re: get_arg_page() && ptr_size accounting

2018-09-11 Thread Oleg Nesterov
On 09/10, Kees Cook wrote: > > On Mon, Sep 10, 2018 at 10:21 AM, Oleg Nesterov wrote: > > > Could you explain what this patch actually prevents from? Especially > > now that we have stack_guard_gap? > > One of the many Stack Clash abuses was that it was possible to jump > over the stack gap with

Re: get_arg_page() && ptr_size accounting

2018-09-11 Thread Oleg Nesterov
On 09/10, Kees Cook wrote: > > I've looked more closely now. So, while I agree with you about > resource limits, there's a corner case that is better handled here: > once we've called flush_old_exec(), we can no longer send errors back > to the parent. We just segfault. So, I think it's better to

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 10:43 AM, Oleg Nesterov wrote: > On 09/10, Oleg Nesterov wrote: >> >> On 09/10, Kees Cook wrote: >> > >> > On Mon, Sep 10, 2018 at 9:41 AM, Kees Cook wrote: >> > > On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: >> > >> Hi Kees, >> > >> >> > >> I was thinking about

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 10:21 AM, Oleg Nesterov wrote: > On 09/10, Kees Cook wrote: >> >> On Mon, Sep 10, 2018 at 9:41 AM, Kees Cook wrote: >> > On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: >> >> Hi Kees, >> >> >> >> I was thinking about backporting the commit 98da7d08850fb8bde >> >>

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 10:18 AM, Oleg Nesterov wrote: > On 09/10, Kees Cook wrote: >> >> > So get_arg_page() does >> > >> > /* >> > * Since the stack will hold pointers to the strings, we >> > * must account for them as well. >> >

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Oleg Nesterov
On 09/10, Oleg Nesterov wrote: > > On 09/10, Kees Cook wrote: > > > > On Mon, Sep 10, 2018 at 9:41 AM, Kees Cook wrote: > > > On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: > > >> Hi Kees, > > >> > > >> I was thinking about backporting the commit 98da7d08850fb8bde > > >> ("fs/exec.c:

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Oleg Nesterov
On 09/10, Kees Cook wrote: > > On Mon, Sep 10, 2018 at 9:41 AM, Kees Cook wrote: > > On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: > >> Hi Kees, > >> > >> I was thinking about backporting the commit 98da7d08850fb8bde > >> ("fs/exec.c: account for argv/envp pointers"), but I am not sure >

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Oleg Nesterov
On 09/10, Kees Cook wrote: > > > 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

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 9:41 AM, Kees Cook wrote: > On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: >> 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... BTW, if you

Re: get_arg_page() && ptr_size accounting

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 5:29 AM, Oleg Nesterov wrote: > 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

get_arg_page() && ptr_size accounting

2018-09-10 Thread Oleg Nesterov
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