Re: [PATCH 5.15.y] fbdev: fix incorrect address computation in deferred IO

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Apr 29, 2024 at 04:40:41PM +0200, Nam Cao wrote:
> commit 78d9161d2bcd442d93d917339297ffa057dbee8c upstream.
> 
> 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@oracle.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@vger.kernel.org>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://patchwork.freedesktop.org/patch/msgid/20240423115053.4490-1-namcao@linutronix.de
> [rebase to v5.15]
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>  drivers/video/fbdev/core/fb_defio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Now queued up, thanks.

greg k-h