On Wed 27-01-21 19:55:38, Tetsuo Handa wrote:
> syzbot is reporting that memdup_user_nul() which receives user-controlled
> size (which can be up to (INT_MAX & PAGE_MASK)) via vfs_write() will hit
> order >= MAX_ORDER path [1].
>
> Making costly allocations (order > PAGE_ALLOC_COSTLY_ORDER) naturally fail
> should be better than trying to enforce PAGE_SIZE upper limit, for some of
> callers accept space-delimited list arguments.
>
> Therefore, let's add __GFP_NOWARN to memdup_user_nul() as with
> commit 6c8fcc096be9d02f ("mm: don't let userspace spam allocations
> warnings"). Also use GFP_USER as with other userspace-controllable
> allocations like memdup_user().
I absolutely detest hiding this behind __GFP_NOWARN. There should be no
reason to even try hard for memdup_user_nul. Can you explain why this
cannot use kvmalloc instead?
> [1] https://syzkaller.appspot.com/bug?id=8bf7efb3db19101b4008dc9198522ef977d098a6
>
> Reported-by: syzbot <syzbot+a71a442385a0b2815497@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> mm/util.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/mm/util.c b/mm/util.c
> index 8c9b7d1e7c49..265b40a86856 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -252,12 +252,7 @@ void *memdup_user_nul(const void __user *src, size_t len)
> {
> char *p;
>
> - /*
> - * Always use GFP_KERNEL, since copy_from_user() can sleep and
> - * cause pagefault, which makes it pointless to use GFP_NOFS
> - * or GFP_ATOMIC.
> - */
> - p = kmalloc_track_caller(len + 1, GFP_KERNEL);
> + p = kmalloc_track_caller(len + 1, GFP_USER | __GFP_NOWARN);
> if (!p)
> return ERR_PTR(-ENOMEM);
>
> --
> 2.18.4
>
--
Michal Hocko
SUSE Labs
