[PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

[PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

[Florian Fuchs]

Add __user annotation to buf, as it is passed as a user pointer in
pin_user_pages_fast(). Use an empty initializer for the sentinel
board-table entry to avoid initializing a function pointer with an
integer literal.

Fixes: 5f566c0ac51c ("fbdev: pvr2fb: Fix leftover reference to ONCHIP_NR_DMA_CHANNELS")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202607131247.fpQ6eTc7-lkp@intel.com/
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
---
Not sure, if we do such changes, but as I received a test robot email, I
leave it up to you all :) (I also compiled and tested the change)

 drivers/video/fbdev/pvr2fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 9428716e2dc4..a6e7abca7a06 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -639,7 +639,7 @@ static irqreturn_t __maybe_unused pvr2fb_interrupt(int irq, void *dev_id)
 }
 
 #ifdef CONFIG_PVR2_DMA
-static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
+static ssize_t pvr2fb_write(struct fb_info *info, const char __user *buf,
 			    size_t count, loff_t *ppos)
 {
 	unsigned long dst, start, end, len;
@@ -1077,7 +1077,7 @@ static struct pvr2_board {
 #ifdef CONFIG_PCI
 	{ pvr2fb_pci_init, pvr2fb_pci_exit, "PCI PVR2" },
 #endif
-	{ 0, },
+	{ },
 };
 
 static int __init pvr2fb_init(void)
-- 
2.43.0

Re: [PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds array read when calculating the `end` address for the contiguity check.
- [High] DMA transfer completely ignores the user buffer's intra-page offset and exact requested length.
- [High] The driver performs hardware DMA operations without any driver-level serialization.
- [High] The boundary check in the per-page DMA fallback loop does not account for the write size, allowing out-of-bounds DMA writes past the end of the mapped video memory.
- [High] An integer overflow in the bounds check calculation allows an attacker to bypass the size check and perform an out-of-bounds DMA write.
- [Medium] `pvr2fb_write` improperly requests `FOLL_WRITE` permissions when pinning the user buffer.
--

--- Patch [1]: [PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer ---
commit 5cca64a0d87619ea95eacae8901003a474bb95a2
Author: Florian Fuchs <fuchsfl@gmail.com>

fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

This commit adds a __user annotation to the buf argument in pvr2fb_write()
and uses an empty initializer for the board-table sentinel.

Link: https://lore.kernel.org/oe-kbuild-all/202607131247.fpQ6eTc7-lkp@intel.com/

> diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
> index 9428716e2dc40..a6e7abca7a06f 100644
> --- a/drivers/video/fbdev/pvr2fb.c
> +++ b/drivers/video/fbdev/pvr2fb.c
> @@ -639,7 +639,7 @@ static irqreturn_t __maybe_unused pvr2fb_interrupt(int irq, void *dev_id)
>  }
>  
>  #ifdef CONFIG_PVR2_DMA
> -static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
> +static ssize_t pvr2fb_write(struct fb_info *info, const char __user *buf,
>  			    size_t count, loff_t *ppos)
>  {
>  	unsigned long dst, start, end, len;

[Severity: Medium]
This isn't a bug introduced by this patch, but since we are modifying the
write function signature to annotate the user buffer, could we also look at
how it is pinned further down in this function?

pvr2fb_write() uses:

    ret = pin_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);

Does requesting FOLL_WRITE here cause valid read-only user buffers (like
string literals) to fail with -EFAULT when passed to write()?

[Severity: High]
This is a pre-existing issue, but further down in pvr2fb_write(), there
appear to be concerns with how the transfer boundaries are calculated:

    start = (unsigned long)page_address(pages[0]);
    end   = (unsigned long)page_address(pages[nr_pages]);
    len   = nr_pages << PAGE_SHIFT;

Since the pages array only has nr_pages elements, wouldn't reading
pages[nr_pages] be an out-of-bounds array read that could panic the kernel?

Additionally, by ignoring the intra-page offset (buf & ~PAGE_MASK) and
calculating the length strictly from nr_pages, might this leak unrelated
memory by copying from the start of the physical page rather than the exact
user buffer offset?

[Severity: High]
This isn't a bug introduced by this patch, but the size check in
pvr2fb_write() seems susceptible to overflow:

    if ((*ppos + len) > fb_info->fix.smem_len) {
        ret = -ENOSPC;
        goto out_unmap;
    }

Given that *ppos is a 64-bit loff_t and len is a 32-bit unsigned int, could
a sufficiently large *ppos cause the addition to wrap around to a negative
value, bypassing the smem_len check and allowing an out-of-bounds DMA write?

[Severity: High]
This is a pre-existing issue, but when setting up the DMA transfers:

    dma_configure_channel(shdma, 0x12c1);
    ...
    dma_write(shdma, start, 0, len);
    dma_write(pvr2dma, 0, dst, len);
    dma_wait_for_completion(pvr2dma);

Are there any locks missing here? It looks like concurrent write() calls
could interleave DMA configurations on the shared shdma and pvr2dma
channels, which might corrupt hardware state.

[Severity: High]
This isn't a bug introduced by this patch, but the fallback loop for
non-contiguous pages has a potential bounds check gap:

    if ((*ppos + (i << PAGE_SHIFT)) > fb_info->fix.smem_len) {
        ret = -ENOSPC;
        goto out_unmap;
    }

    dma_write_page(shdma, (unsigned long)page_address(pages[i]), 0);
    dma_write_page(pvr2dma, 0, dst);

Because the check only verifies the start address (*ppos + (i << PAGE_SHIFT))
without accounting for the PAGE_SIZE transfer size of dma_write_page(), does
this allow an out-of-bounds DMA write of up to 4095 bytes past the end of the
mapped video memory?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713101638.600333-1-fuchsfl@gmail.com?part=1

Re: [PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

[Helge Deller]

On 7/13/26 12:16, Florian Fuchs wrote:
> Add __user annotation to buf, as it is passed as a user pointer in
> pin_user_pages_fast(). Use an empty initializer for the sentinel
> board-table entry to avoid initializing a function pointer with an
> integer literal.
> 
> Fixes: 5f566c0ac51c ("fbdev: pvr2fb: Fix leftover reference to ONCHIP_NR_DMA_CHANNELS")

I think this Fixes tag is wrong.
The problem has been there forever.

> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202607131247.fpQ6eTc7-lkp@intel.com/
> Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
> ---
> Not sure, if we do such changes, but as I received a test robot email, I
> leave it up to you all :) (I also compiled and tested the change)
> 
>   drivers/video/fbdev/pvr2fb.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
Patch applied to fbdev git tree.

Thanks for fixing!
Helge

Re: [PATCH] fbdev: pvr2fb: correct user pointer annotation and sentinel initializer

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Mon, Jul 13, 2026 at 12:16:38PM +0200, Florian Fuchs wrote:
> Add __user annotation to buf, as it is passed as a user pointer in
> pin_user_pages_fast(). Use an empty initializer for the sentinel
> board-table entry to avoid initializing a function pointer with an
> integer literal.

FTR: Using 0 in an initializer is covered by the C standard. Anyhow, I
agree that { } is nicer.

Having said that, I think the right thing to do here is to drop the
sentinel entry, because board_driver[] is only used in for loops that
iterate between 0 (inclusive) and ARRAY_SIZE(board_driver) (exclusive).

Also I would split this patch in two.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]