| Message ID | 83ed0748-634d-4146-d216-53681bc3b553@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: usb/cpia2: fix start_offset+size Integer Overflow in, cpia2_remap_buffer | expand |
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
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 > > . >
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
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. > . >
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
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;