* [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
@ 2019-12-11 2:47 Zhiqiang Liu
2019-12-11 7:57 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Zhiqiang Liu @ 2019-12-11 2:47 UTC (permalink / raw)
To: tglx, crope, linux-media, rfontana, gregkh, erik.andren, hverkuil,
brijohn, lcostantino, hverkuil, suweifeng1
Cc: Mingfangsen, guiyao
From: Weifeng Su <suweifeng1@huawei.com>
CVE-2019-18675: The Linux kernel through 5.3.13 has a start_offset+size
IntegerOverflow in cpia2_remap_buffer in drivers/media/usb/cpia2/cpia2_core.c
because cpia2 has its own mmap implementation. This allows local users
(with /dev/video0 access) to obtain read and write permissions on kernel
physical pages, which can possibly result in a privilege escalation.
Here, we fix it through proper start_offset value check.
CVE Link: https://nvd.nist.gov/vuln/detail/CVE-2019-18675
Signed-off-by: Weifeng Su <suweifeng1@huawei.com>
Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
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;
--
2.24.0.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
2019-12-11 2:47 [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer Zhiqiang Liu
@ 2019-12-11 7:57 ` Greg KH
2019-12-12 1:48 ` Zhiqiang Liu
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-12-11 7:57 UTC (permalink / raw)
To: Zhiqiang Liu
Cc: tglx, crope, linux-media, rfontana, erik.andren, hverkuil,
brijohn, lcostantino, suweifeng1, Mingfangsen, guiyao
On Wed, Dec 11, 2019 at 10:47:58AM +0800, Zhiqiang Liu wrote:
> From: Weifeng Su <suweifeng1@huawei.com>
>
> CVE-2019-18675: The Linux kernel through 5.3.13 has a start_offset+size
> IntegerOverflow in cpia2_remap_buffer in drivers/media/usb/cpia2/cpia2_core.c
> because cpia2 has its own mmap implementation. This allows local users
> (with /dev/video0 access) to obtain read and write permissions on kernel
> physical pages, which can possibly result in a privilege escalation.
>
> Here, we fix it through proper start_offset value check.
>
> CVE Link: https://nvd.nist.gov/vuln/detail/CVE-2019-18675
> Signed-off-by: Weifeng Su <suweifeng1@huawei.com>
> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> 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))
I thought we discussed this already, and the checks in the core kernel
will prevent this from happening, right?
What did I miss?
Or was that research not correct? Can you really trigger this? If so,
we should fix the core kernel checks instead, and not rely on it being
in every individual driver.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
2019-12-11 7:57 ` Greg KH
@ 2019-12-12 1:48 ` Zhiqiang Liu
2019-12-12 7:47 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Zhiqiang Liu @ 2019-12-12 1:48 UTC (permalink / raw)
To: Greg KH
Cc: tglx, crope, linux-media, rfontana, erik.andren, hverkuil,
brijohn, lcostantino, suweifeng1, Mingfangsen, guiyao
On 2019/12/11 15:57, Greg KH wrote:
> On Wed, Dec 11, 2019 at 10:47:58AM +0800, Zhiqiang Liu wrote:
>> From: Weifeng Su <suweifeng1@huawei.com>
>>
>> CVE-2019-18675: The Linux kernel through 5.3.13 has a start_offset+size
>> IntegerOverflow in cpia2_remap_buffer in drivers/media/usb/cpia2/cpia2_core.c
>> because cpia2 has its own mmap implementation. This allows local users
>> (with /dev/video0 access) to obtain read and write permissions on kernel
>> physical pages, which can possibly result in a privilege escalation.
>>
>> Here, we fix it through proper start_offset value check.
>>
>> CVE Link: https://nvd.nist.gov/vuln/detail/CVE-2019-18675
>> Signed-off-by: Weifeng Su <suweifeng1@huawei.com>
>> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> ---
>> 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))
>
> I thought we discussed this already, and the checks in the core kernel
> will prevent this from happening, right?
> What did I miss?
>
Thanks for your reply.
It is me who missed the discussion. Could you sent me the mails or links about
the discussion?
> Or was that research not correct? Can you really trigger this? If so,
> we should fix the core kernel checks instead, and not rely on it being
> in every individual driver.
>
> thanks,
>
Omer Shalev have given a example which can trigger the CVE.
Example link: https://deshal3v.github.io/blog/kernel-research/mmap_exploitation
> greg k-h
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
2019-12-12 1:48 ` Zhiqiang Liu
@ 2019-12-12 7:47 ` Greg KH
2019-12-12 9:40 ` Zhiqiang Liu
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-12-12 7:47 UTC (permalink / raw)
To: Zhiqiang Liu
Cc: tglx, crope, linux-media, rfontana, erik.andren, hverkuil,
brijohn, lcostantino, suweifeng1, Mingfangsen, guiyao
On Thu, Dec 12, 2019 at 09:48:44AM +0800, Zhiqiang Liu wrote:
> On 2019/12/11 15:57, Greg KH wrote:
> > On Wed, Dec 11, 2019 at 10:47:58AM +0800, Zhiqiang Liu wrote:
> >> From: Weifeng Su <suweifeng1@huawei.com>
> >>
> >> CVE-2019-18675: The Linux kernel through 5.3.13 has a start_offset+size
> >> IntegerOverflow in cpia2_remap_buffer in drivers/media/usb/cpia2/cpia2_core.c
> >> because cpia2 has its own mmap implementation. This allows local users
> >> (with /dev/video0 access) to obtain read and write permissions on kernel
> >> physical pages, which can possibly result in a privilege escalation.
> >>
> >> Here, we fix it through proper start_offset value check.
> >>
> >> CVE Link: https://nvd.nist.gov/vuln/detail/CVE-2019-18675
> >> Signed-off-by: Weifeng Su <suweifeng1@huawei.com>
> >> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> >> ---
> >> 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))
> >
> > I thought we discussed this already, and the checks in the core kernel
> > will prevent this from happening, right?
> > What did I miss?
> >
> Thanks for your reply.
> It is me who missed the discussion. Could you sent me the mails or links about
> the discussion?
See Omer's email thread on the linux-kernel mailing list where he asks
about this very issue. You can find it in the archives:
https://lore.kernel.org/lkml/20191108215038.59170-1-omerdeshalev@gmail.com/
> > Or was that research not correct? Can you really trigger this? If so,
> > we should fix the core kernel checks instead, and not rely on it being
> > in every individual driver.
> >
> > thanks,
> >
> Omer Shalev have given a example which can trigger the CVE.
> Example link: https://deshal3v.github.io/blog/kernel-research/mmap_exploitation
That "example" was run on a kernel without the above mentioned commit to
fix all of this.
Have you tried this on the latest kernel release and succeeded?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
2019-12-12 7:47 ` Greg KH
@ 2019-12-12 9:40 ` Zhiqiang Liu
2019-12-12 10:01 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Zhiqiang Liu @ 2019-12-12 9:40 UTC (permalink / raw)
To: Greg KH
Cc: tglx, crope, linux-media, rfontana, erik.andren, hverkuil,
brijohn, lcostantino, suweifeng1, Mingfangsen, guiyao
On 2019/12/12 15:47, Greg KH wrote:
> On Thu, Dec 12, 2019 at 09:48:44AM +0800, Zhiqiang Liu wrote:
>> Omer Shalev have given a example which can trigger the CVE.
>> Example link: https://deshal3v.github.io/blog/kernel-research/mmap_exploitation
>
> That "example" was run on a kernel without the above mentioned commit to
> fix all of this.
>
> Have you tried this on the latest kernel release and succeeded?
>
> thanks,
>
> greg k-h
>
Thanks for patiently answering my question.
Actually, I have missed the Omer's commit.
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer
2019-12-12 9:40 ` Zhiqiang Liu
@ 2019-12-12 10:01 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-12-12 10:01 UTC (permalink / raw)
To: Zhiqiang Liu
Cc: tglx, crope, linux-media, rfontana, erik.andren, hverkuil,
brijohn, lcostantino, suweifeng1, Mingfangsen, guiyao
On Thu, Dec 12, 2019 at 05:40:54PM +0800, Zhiqiang Liu wrote:
>
>
> On 2019/12/12 15:47, Greg KH wrote:
> > On Thu, Dec 12, 2019 at 09:48:44AM +0800, Zhiqiang Liu wrote:
> >> Omer Shalev have given a example which can trigger the CVE.
> >> Example link: https://deshal3v.github.io/blog/kernel-research/mmap_exploitation
> >
> > That "example" was run on a kernel without the above mentioned commit to
> > fix all of this.
> >
> > Have you tried this on the latest kernel release and succeeded?
> >
> > thanks,
> >
> > greg k-h
> >
>
> Thanks for patiently answering my question.
> Actually, I have missed the Omer's commit.
As pennance, you can go revoke that CVE so this doesn't come up again in
a few months from someone else :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-12 10:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-11 2:47 [PATCH] media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer Zhiqiang Liu
2019-12-11 7:57 ` Greg KH
2019-12-12 1:48 ` Zhiqiang Liu
2019-12-12 7:47 ` Greg KH
2019-12-12 9:40 ` Zhiqiang Liu
2019-12-12 10:01 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox