linux-kernel - Re: [PATCH] Properly check tee

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAHUa44HB6WKoubGEWFANh8SmsOU0Z=p+42AV5-At8TC1B08npA@mail.gmail.com>
Date:   Mon, 23 Nov 2020 14:52:55 +0100
From:   Jens Wiklander <jens.wiklander@...aro.org>
To:     gaoyusong <a869920004@....com>
Cc:     Sumit Semwal <sumit.semwal@...aro.org>,
        Christian Koenig <christian.koenig@....com>,
        op-tee@...ts.trustedfirmware.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-media@...r.kernel.org,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH] Properly check tee_shm buffer mmap offset

Hi,

On Mon, Nov 23, 2020 at 8:10 AM gaoyusong <a869920004@....com> wrote:
>
> The memmap options in tee_shm_op_mmap were not being checked for all
> sets of possible crazy values. Fix this up by properly check tee_shm
> buffer offsets.
>
> Signed-off-by: gaoyusong <a869920004@....com>
> ---
>  drivers/tee/tee_shm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 827ac3d..3f762c8 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -75,6 +75,16 @@ static int tee_shm_op_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
>  {
>         struct tee_shm *shm = dmabuf->priv;
>         size_t size = vma->vm_end - vma->vm_start;
> +       unsigned long offset;
> +
> +       /* Check dmabuffer mmap offset */
> +       if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
> +               return -EINVAL;
> +
> +       offset = vma->vm_pgoff << PAGE_SHIFT;
> +
> +       if (offset > shm->size || size > shm->size - offset)
> +               return -EINVAL;

If we would have used vm_pgoff below to offset into the shm buffer
these checks would be needed.
Currently we're ignoring this field though. That might be a bit
inconsistent with the mmap() API, but on the other hand this buffer
has just been carved out of the shared memory pool for the purpose of
mapping it in user space. To carve out more than we're going to map
would be wasteful so I guess that in the end it makes sense to ignore
vm_pgoff.

Thanks,
Jens

>
>         /* Refuse sharing shared memory provided by application */
>         if (shm->flags & TEE_SHM_USER_MAPPED)
> --
> 1.8.3.1
>