Re: [PATCH] Protection for exploiting null dereference using mmap

[Chris Wright <chrisw@sous-sol.org>]

* Eric Paris (eparis@redhat.com) wrote:
> +mmap_protect_memory

I'm terrible at names, but something like mmap_minimum_addr would be a
little clearer at describing that it's a lower bound on mapping memory.
BTW, this is also an arch specific issue, those with disjoint kernel
and user memory space don't suffer (yet another reason to default to 0).

> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -615,6 +615,15 @@ static ctl_table kern_table[] = {
>  		.proc_handler   = &proc_dointvec,
>  	},
>  #endif
> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "mmap_protect_memory",
> +		.data		= &mmap_protect_memory,
> +		.maxlen         = sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec,
> +		.strategy	= &sysctl_intvec,

I don't think this strategy does anything without some boundary values.

> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -291,6 +291,10 @@ unsigned long do_mremap(unsigned long addr,
>  		if ((addr <= new_addr) && (addr+old_len) > new_addr)
>  			goto out;
>  
> +		ret = security_file_mmap(0, 0, 0, 0, new_addr, 1);
> +		if (ret)
> +			goto out;
> +
>  		ret = do_munmap(mm, new_addr, new_len);
>  		if (ret)
>  			goto out;
> @@ -390,9 +394,16 @@ unsigned long do_mremap(unsigned long addr,
>  
>  			new_addr = get_unmapped_area(vma->vm_file, 0, new_len,
>  						vma->vm_pgoff, map_flags);
> -			ret = new_addr;
> -			if (new_addr & ~PAGE_MASK)
> +			if (new_addr & ~PAGE_MASK) {
> +				ret = new_addr;
>  				goto out;
> +			}
> +
> +			ret = security_file_mmap(0, 0, 0, 0, new_addr, 1);
> +			if (ret)
> +				goto out;
> +
> +			ret = new_addr;

Nit: unnecessary assignment...

>  		}
>  		ret = move_vma(vma, addr, old_len, new_len, new_addr);
		^^^
...as it's overwritten immediately after.