linux-kernel - Re: [PATCH] media: cpia2: Fix integer overflow in mmap handling

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Jan 2020 15:02:32 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Takashi Iwai <tiwai@...e.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: cpia2: Fix integer overflow in mmap handling

Hi Takashi,

On 1/8/20 5:16 PM, Takashi Iwai wrote:
> The offset and size checks in cpia2_regmap_buffer() may ignore the
> integer overflow and allow local users to obtain the access to the
> kernel physical pages.
> 
> Fix it by modifying the check more carefully; the size value is
> already checked beforehand and guaranteed to be smaller than
> cam->frame_size*num_frames, so it's safe to subtract in the right
> hand side.
> 
> This covers CVE-2019-18675.
> 
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@...e.de>
> ---
> 
> I'm submitting this since there hasn't been any action seen for this
> bug over a month.  Let me know if there is already a fix.  Thanks.

Read the full mail thread for the original patches:

https://patchwork.linuxtv.org/patch/60602/
https://patchwork.linuxtv.org/patch/59978/

The second has the reference to the kernel core mmap commit that prevents this
from being exploited.

Rejecting this patch for that reason.

Since this is the third time this patch pops up, I am wondering if I shouldn't
accept it anyway just to stop this. But then I want a better commit log that
points to the core commit as the *real* fix.

There is nothing wrong as such with this patch, so if someone cares to post
a new version that refers to the core commit, I'll likely accept it.

Regards,

	Hans

> 
> 
>  drivers/media/usb/cpia2/cpia2_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/cpia2/cpia2_core.c b/drivers/media/usb/cpia2/cpia2_core.c
> index 20c50c2d042e..26ae7a5e3783 100644
> --- a/drivers/media/usb/cpia2/cpia2_core.c
> +++ b/drivers/media/usb/cpia2/cpia2_core.c
> @@ -2401,7 +2401,7 @@ int cpia2_remap_buffer(struct camera_data *cam, struct vm_area_struct *vma)
>  
>  	if (size > cam->frame_size*cam->num_frames  ||
>  	    (start_offset % cam->frame_size) != 0 ||
> -	    (start_offset+size > cam->frame_size*cam->num_frames))
> +	    (start_offset > cam->frame_size*cam->num_frames - size))
>  		return -EINVAL;
>  
>  	pos = ((unsigned long) (cam->frame_buffer)) + start_offset;
>