Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The patch replaces direct access to `map->vaddr` with `iosys_map_get_vaddr()`. However, `iosys_map_get_vaddr()` explicitly warns and returns `NULL` if the mapping points to I/O memory (`map->is_iomem == true`). When userspace imports a dma-buf from a discrete GPU (which typically exports VRAM as I/O memory) into the UDL driver and uses it as a framebuffer, the damage handler `udl_handle_damage()` will receive an I/O memory mapping. Calling `iosys_map_get_vaddr()` on this mapping triggers a `WARN_ON` for every damage rectangle processed and fails the update by returning `-EFAULT`. This functionally breaks PRIME imports from discrete GPUs to DisplayLink devices and creates a local Denial of Service (DoS) vector by allowing a local user to spam the kernel log with warnings or trigger a panic if `panic_on_warn` is enabled. The correct fix for the original architectural issue should have been using `iosys_map_rd()` or `iosys_map_memcpy_from()` rather than `iosys_map_get_vaddr()`. --
--- Patch [2]: [PATCH 2/2] drm/udl: Use iosys_map getter for virtual address --- commit 935887dfbb84bad699c0f9cf22dbe263fac86a93 Author: Shixiong Ou <[email protected]> drm/udl: Use iosys_map getter for virtual address Replace direct access to map->vaddr with the new iosys_map_get_vaddr() helper function. This properly uses the iosys_map abstraction layer instead of directly accessing structure members. > diff --git a/drivers/gpu/drm/udl/udl_modeset.c > b/drivers/gpu/drm/udl/udl_modeset.c > index 289711035b67c..06a3a4d149b9d 100644 > --- a/drivers/gpu/drm/udl/udl_modeset.c > +++ b/drivers/gpu/drm/udl/udl_modeset.c > @@ -207,12 +207,16 @@ static int udl_handle_damage(struct drm_framebuffer *fb, > { > struct drm_device *dev = fb->dev; > struct udl_device *udl = to_udl(dev); > - void *vaddr = map->vaddr; /* TODO: Use mapping abstraction properly */ > + void *vaddr; > int i, ret; > char *cmd; > struct urb *urb; > int log_bpp; > > + vaddr = iosys_map_get_vaddr(map); > + if (!vaddr) > + return -EFAULT; > + [Severity: High] Does this code break PRIME imports from discrete GPUs? If userspace imports a dma-buf from a discrete GPU (which typically exports VRAM as I/O memory) into the UDL driver and uses it as a framebuffer, the map passed to udl_handle_damage() will be mapped as IOMEM. Looking at iosys_map_get_vaddr() in include/linux/iosys-map.h: static inline void *iosys_map_get_vaddr(const struct iosys_map *map) { if (WARN_ON(map->is_iomem)) return NULL; return map->vaddr; } This will trigger a WARN_ON for every damage rectangle processed and return -EFAULT, failing the update. Could this create a local DoS if panic_on_warn is enabled, or spam the kernel log with warnings? Would it be safer to use iosys_map_rd() or iosys_map_memcpy_from() here rather than iosys_map_get_vaddr() to properly handle IOMEM mappings? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
