On Mon, Jan 27, 2020 at 03:56:16PM -0800, Todd Kjos wrote:
> From: Suren Baghdasaryan <surenb@xxxxxxxxxx>
>
> When ashmem file is mmapped, the resulting vma->vm_file points to the
> backing shmem file with the generic fops that do not check ashmem
> permissions like fops of ashmem do. If an mremap is done on the ashmem
> region, then the permission checks will be skipped. Fix that by disallowing
> mapping operation on the backing shmem file.
Reviewed-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
thanks!
- Joel
>
> Reported-by: Jann Horn <jannh@xxxxxxxxxx>
> Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxxxxxxx> # 4.4,4.9,4.14,4.18,5.4
> Signed-off-by: Todd Kjos <tkjos@xxxxxxxxxx>
> ---
> drivers/staging/android/ashmem.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> v2: update commit message as suggested by joelaf@xxxxxxxxxx.
>
> diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
> index 74d497d39c5a..c6695354b123 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -351,8 +351,23 @@ static inline vm_flags_t calc_vm_may_flags(unsigned long prot)
> _calc_vm_trans(prot, PROT_EXEC, VM_MAYEXEC);
> }
>
> +static int ashmem_vmfile_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + /* do not allow to mmap ashmem backing shmem file directly */
> + return -EPERM;
> +}
> +
> +static unsigned long
> +ashmem_vmfile_get_unmapped_area(struct file *file, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags)
> +{
> + return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
> +}
> +
> static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
> {
> + static struct file_operations vmfile_fops;
> struct ashmem_area *asma = file->private_data;
> int ret = 0;
>
> @@ -393,6 +408,19 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
> }
> vmfile->f_mode |= FMODE_LSEEK;
> asma->file = vmfile;
> + /*
> + * override mmap operation of the vmfile so that it can't be
> + * remapped which would lead to creation of a new vma with no
> + * asma permission checks. Have to override get_unmapped_area
> + * as well to prevent VM_BUG_ON check for f_ops modification.
> + */
> + if (!vmfile_fops.mmap) {
> + vmfile_fops = *vmfile->f_op;
> + vmfile_fops.mmap = ashmem_vmfile_mmap;
> + vmfile_fops.get_unmapped_area =
> + ashmem_vmfile_get_unmapped_area;
> + }
> + vmfile->f_op = &vmfile_fops;
> }
> get_file(asma->file);
>
> --
> 2.25.0.341.g760bfbb309-goog
>
