[PATCH 1/2] iosys-map: Add getter functions for virtual addresses

[PATCH 1/2] iosys-map: Add getter functions for virtual addresses

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

Add iosys_map_get_vaddr() and iosys_map_get_vaddr_iomem() as the
counterparts to iosys_map_set_vaddr() and iosys_map_set_vaddr_iomem().

This allows drivers to use the iosys_map abstraction layer properly
instead of directly accessing the structure members, improving code
maintainability and safety.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
 include/linux/iosys-map.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
index 3e85afe794c0..658d5038498a 100644
--- a/include/linux/iosys-map.h
+++ b/include/linux/iosys-map.h
@@ -200,6 +200,42 @@ static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
 	map->is_iomem = true;
 }
 
+/**
+ * iosys_map_get_vaddr - Gets the virtual address of a system-memory mapping
+ * @map:	The iosys_map structure
+ *
+ * Returns the system-memory address. The caller must ensure the mapping
+ * refers to system memory (not I/O memory).
+ *
+ * Returns:
+ * The system-memory address, or NULL if the mapping is not set or refers
+ * to I/O memory.
+ */
+static inline void *iosys_map_get_vaddr(const struct iosys_map *map)
+{
+	if (WARN_ON(map->is_iomem))
+		return NULL;
+	return map->vaddr;
+}
+
+/**
+ * iosys_map_get_vaddr_iomem - Gets the virtual address of an I/O-memory mapping
+ * @map:	The iosys_map structure
+ *
+ * Returns the I/O-memory address. The caller must ensure the mapping
+ * refers to I/O memory (not system memory).
+ *
+ * Returns:
+ * The I/O-memory address, or NULL if the mapping is not set or refers
+ * to system memory.
+ */
+static inline void __iomem *iosys_map_get_vaddr_iomem(const struct iosys_map *map)
+{
+	if (WARN_ON(!map->is_iomem))
+		return NULL;
+	return map->vaddr_iomem;
+}
+
 /**
  * iosys_map_is_equal - Compares two iosys mapping structures for equality
  * @lhs:	The iosys_map structure
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH 2/2] drm/udl: Use iosys_map getter for virtual address

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

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.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
 drivers/gpu/drm/udl/udl_modeset.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index 289711035b67..06a3a4d149b9 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;
+
 	ret = udl_log_cpp(fb->format->cpp[0]);
 	if (ret < 0)
 		return ret;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH 2/2] drm/udl: Use iosys_map getter for virtual address

[sashiko-bot]

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 <oushixiong@kylinos.cn>

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/20260724093808.776138-1-oushixiong1025@163.com?part=2

Re: [PATCH 1/2] iosys-map: Add getter functions for virtual addresses

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] iosys_map_get_vaddr_iomem() unconditionally triggers a kernel warning (WARN_ON) when called on an unset mapping, contradicting its documentation.
- [Low] The patch adds `iosys_map_get_vaddr` to `include/linux/iosys-map.h` but fails to remove the pre-existing static inline definition in `drivers/firmware/tegra/ivc.c`, causing a build failure.
- [Low] Missing <linux/bug.h> include for WARN_ON(), leading to potential build breaks.
--

--- Patch [1]: [PATCH 1/2] iosys-map: Add getter functions for virtual addresses ---
commit f14cef96efd32780d25ba0c941cea2c91611ddcd
Author: Shixiong Ou <oushixiong@kylinos.cn>

iosys-map: Add getter functions for virtual addresses

Add iosys_map_get_vaddr() and iosys_map_get_vaddr_iomem() as the
counterparts to iosys_map_set_vaddr() and iosys_map_set_vaddr_iomem().

This allows drivers to use the iosys_map abstraction layer properly
instead of directly accessing the structure members, improving code
maintainability and safety.

> diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
> index 3e85afe794c0a..658d5038498a7 100644
> --- a/include/linux/iosys-map.h
> +++ b/include/linux/iosys-map.h
> @@ -200,6 +200,42 @@ static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
>  	map->is_iomem = true;
>  }
>  
> +/**
> + * iosys_map_get_vaddr - Gets the virtual address of a system-memory mapping
> + * @map:	The iosys_map structure
> + *
> + * Returns the system-memory address. The caller must ensure the mapping
> + * refers to system memory (not I/O memory).
> + *
> + * Returns:
> + * The system-memory address, or NULL if the mapping is not set or refers
> + * to I/O memory.
> + */
> +static inline void *iosys_map_get_vaddr(const struct iosys_map *map)
> +{
> +	if (WARN_ON(map->is_iomem))
> +		return NULL;

[Severity: Low]
Does this new usage of WARN_ON() require adding an include for <linux/bug.h>?

Files including <linux/iosys-map.h> that don't already include <linux/bug.h>
might fail to compile due to implicit declaration of the function.

[Severity: Low]
Could this conflict with the existing static inline definition of
iosys_map_get_vaddr() in drivers/firmware/tegra/ivc.c?

drivers/firmware/tegra/ivc.c already defines this function locally:

static inline void *iosys_map_get_vaddr(const struct iosys_map *map)
{
	if (WARN_ON(map->is_iomem))
		return NULL;
	return map->vaddr;
}

It appears drivers/firmware/tegra/ivc.c might encounter a build failure due
to function redefinition since it includes <linux/iosys-map.h> through
<soc/tegra/ivc.h>.

> +	return map->vaddr;
> +}
> +
> +/**
> + * iosys_map_get_vaddr_iomem - Gets the virtual address of an I/O-memory mapping
> + * @map:	The iosys_map structure
> + *
> + * Returns the I/O-memory address. The caller must ensure the mapping
> + * refers to I/O memory (not system memory).
> + *
> + * Returns:
> + * The I/O-memory address, or NULL if the mapping is not set or refers
> + * to system memory.
> + */
> +static inline void __iomem *iosys_map_get_vaddr_iomem(const struct iosys_map *map)
> +{
> +	if (WARN_ON(!map->is_iomem))

[Severity: Medium]
Will this unconditionally trigger a kernel warning for an unset mapping?

The kernel-doc for this function says it returns NULL if the mapping is not
set. However, if iosys_map_clear() is called, the structure is zeroed and
map->is_iomem becomes false.

Calling iosys_map_get_vaddr_iomem() on this cleared mapping would then evaluate
!map->is_iomem as true and trigger the WARN_ON.

> +		return NULL;
> +	return map->vaddr_iomem;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724093808.776138-1-oushixiong1025@163.com?part=1

Re: [PATCH 2/2] drm/udl: Use iosys_map getter for virtual address

[Thomas Zimmermann]

Hi

Am 24.07.26 um 11:38 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> 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.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
>   drivers/gpu/drm/udl/udl_modeset.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> index 289711035b67..06a3a4d149b9 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;
> +

This doesn't work in practice because we don't handle the vmap_iomem 
correctly. You'll get NULL now, when before it was treated as a regular 
address.

The current type aliasing works until we have proper import handling of 
I/O-mem addresses.

Best regards
Thomas


>   	ret = udl_log_cpp(fb->format->cpp[0]);
>   	if (ret < 0)
>   		return ret;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)