[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2bf36b5-3145-491b-b272-d5afae73db7f@oracle.com>
Date: Tue, 23 Apr 2024 18:56:52 +0530
From: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
To: Thomas Zimmermann <tzimmermann@...e.de>, Nam Cao <namcao@...utronix.de>,
Jaya Kumar <jayalk@...works.biz>, Daniel Vetter <daniel@...ll.ch>,
Helge Deller <deller@....de>,
Javier Martinez Canillas <javierm@...hat.com>,
linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Cc: tiwai@...e.de, bigeasy@...utronix.de, patrik.r.jakobsson@...il.com,
Vegard Nossum <vegard.nossum@...cle.com>,
George Kennedy <george.kennedy@...cle.com>,
Darren Kenny <darren.kenny@...cle.com>, chuansheng.liu@...el.com,
stable@...r.kernel.org
Subject: Re: [PATCH v2] fbdev: fix incorrect address computation in deferred
IO
Hi,
On 23/04/24 18:37, Thomas Zimmermann wrote:
> Hi
>
> Am 23.04.24 um 13:50 schrieb Nam Cao:
>> With deferred IO enabled, a page fault happens when data is written to
>> the
>> framebuffer device. Then driver determines which page is being updated by
>> calculating the offset of the written virtual address within the virtual
>> memory area, and uses this offset to get the updated page within the
>> internal buffer. This page is later copied to hardware (thus the name
>> "deferred IO").
>>
>> This offset calculation is only correct if the virtual memory area is
>> mapped to the beginning of the internal buffer. Otherwise this is wrong.
>> For example, if users do:
>> mmap(ptr, 4096, PROT_WRITE, MAP_FIXED | MAP_SHARED, fd, 0xff000);
>>
>> Then the virtual memory area will mapped at offset 0xff000 within the
>> internal buffer. This offset 0xff000 is not accounted for, and wrong page
>> is updated.
>>
>> Correct the calculation by using vmf->pgoff instead. With this change,
>> the
>> variable "offset" will no longer hold the exact offset value, but it is
>> rounded down to multiples of PAGE_SIZE. But this is still correct,
>> because
>> this variable is only used to calculate the page offset.
>>
>> Reported-by: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
>> Closes:
>> https://lore.kernel.org/linux-fbdev/271372d6-e665-4e7f-b088-dee5f4ab341a@oracle.com
>> Fixes: 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref struct")
>> Cc: <stable@...r.kernel.org>
>> Signed-off-by: Nam Cao <namcao@...utronix.de>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
>
Thanks everyone!
I have tested the patched kernel with the syzkaller reproducer and
couldn't see a problem.
Regards,
Harshit
> Thank you so much. I'll take care of merging the patch later this week.
>
> Best regards
> Thomas
>
>> ---
>> v2:
>> - simplify the patch by using vfg->pgoff
>> - remove tested-by tag, as the patch is now different
>>
>> drivers/video/fbdev/core/fb_defio.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fb_defio.c
>> b/drivers/video/fbdev/core/fb_defio.c
>> index 1ae1d35a5942..b9607d5a370d 100644
>> --- a/drivers/video/fbdev/core/fb_defio.c
>> +++ b/drivers/video/fbdev/core/fb_defio.c
>> @@ -196,7 +196,7 @@ static vm_fault_t fb_deferred_io_track_page(struct
>> fb_info *info, unsigned long
>> */
>> static vm_fault_t fb_deferred_io_page_mkwrite(struct fb_info *info,
>> struct vm_fault *vmf)
>> {
>> - unsigned long offset = vmf->address - vmf->vma->vm_start;
>> + unsigned long offset = vmf->pgoff << PAGE_SHIFT;
>> struct page *page = vmf->page;
>> file_update_time(vmf->vma->vm_file);
>
Powered by blists - more mailing lists