[PATCH] drm/vmwgfx: do not hand the embedded sg_table to the PRIME core

[PATCH] drm/vmwgfx: do not hand the embedded sg_table to the PRIME core

[Aldo Ariel Panzardo]

vmw_gem_object_get_sg_table() returns vmw_tt->vsgt.sgt when the buffer
object has already been DMA mapped.  vmw_ttm_map_dma() sets that field
to &vmw_tt->sgt, which is a member embedded inside the struct
vmw_ttm_tt allocation and not a table of its own.

The core owns whatever .get_sg_table returns.  drm_gem_map_dma_buf()
takes the table and drm_gem_unmap_dma_buf() destroys it in full:

	dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC);
	sg_free_table(sgt);
	kfree(sgt);

Both are ops of drm_gem_prime_dmabuf_ops, so the core ends up calling
kfree() on &vmw_tt->sgt, which is not the start of an allocation.  The
preceding sg_free_table() also destroys a scatterlist that vmwgfx still
considers its own and frees again from vmw_ttm_unmap_dma().

drm_gem_unmap_dma_buf() runs from dma_buf_detach(), including the
importer's error path, so a failed import is enough to reach it:
exporting a bound buffer with DRM_IOCTL_PRIME_HANDLE_TO_FD and
importing it on a second DRM device with DRM_IOCTL_PRIME_FD_TO_HANDLE
is sufficient.  Both ioctls are DRM_RENDER_ALLOW.

Observed on 6.12.101 with KASAN, as uid 65534:

  BUG: KASAN: invalid-free in drm_gem_unmap_dma_buf+0xb3/0xf0
  Free of addr ffff888008290450 by task poc_mm04_sgtabl/321
  CPU: 1 UID: 65534 PID: 321 Comm: poc_mm04_sgtabl Not tainted 6.12.101 #4
   kasan_report_invalid_free+0x94/0xc0
   check_slab_allocation+0x116/0x120
   kfree+0x103/0x360
   drm_gem_unmap_dma_buf+0xb3/0xf0
   dma_buf_detach+0x165/0x510
   drm_gem_prime_import_dev+0x33e/0x430
  which belongs to the cache kmalloc-192 of size 192
  The buggy address is located 80 bytes inside of
   144-byte region [ffff888008290400, ffff888008290490)

80 is offsetof(struct vmw_ttm_tt, sgt) and 144 is sizeof(struct
vmw_ttm_tt), which identifies the freed pointer as the embedded member.

Always return a table the core can own, as the other drivers do.
Reusing the cached representation would require copying it into a
freshly allocated sg_table, never returning the alias.

Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 39f8c4655..a0233729b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -73,10 +73,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct drm_gem_object *obj)
 	struct vmw_ttm_tt *vmw_tt =
 		container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm);
 
-	if (vmw_tt->vsgt.sgt)
-		return vmw_tt->vsgt.sgt;
-
-	return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages, vmw_tt->dma_ttm.num_pages);
+	/*
+	 * Do not return &vmw_tt->sgt: the core owns what this returns and
+	 * drm_gem_unmap_dma_buf() sg_free_table()s and kfree()s it, but that
+	 * sg_table is embedded in the vmw_ttm_tt allocation.
+	 */
+	return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
+				     vmw_tt->dma_ttm.num_pages);
 }
 
 static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
-- 
2.43.0

Re: [PATCH] drm/vmwgfx: do not hand the embedded sg_table to the PRIME core

[Zack Rusin]

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

On Wed, Sep 23, 2026 at 8:26 AM Aldo Ariel Panzardo <qwe.aldo@gmail.com> wrote:
>
> vmw_gem_object_get_sg_table() returns vmw_tt->vsgt.sgt when the buffer
> object has already been DMA mapped.  vmw_ttm_map_dma() sets that field
> to &vmw_tt->sgt, which is a member embedded inside the struct
> vmw_ttm_tt allocation and not a table of its own.
>
> The core owns whatever .get_sg_table returns.  drm_gem_map_dma_buf()
> takes the table and drm_gem_unmap_dma_buf() destroys it in full:
>
>         dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC);
>         sg_free_table(sgt);
>         kfree(sgt);
>
> Both are ops of drm_gem_prime_dmabuf_ops, so the core ends up calling
> kfree() on &vmw_tt->sgt, which is not the start of an allocation.  The
> preceding sg_free_table() also destroys a scatterlist that vmwgfx still
> considers its own and frees again from vmw_ttm_unmap_dma().
>
> drm_gem_unmap_dma_buf() runs from dma_buf_detach(), including the
> importer's error path, so a failed import is enough to reach it:
> exporting a bound buffer with DRM_IOCTL_PRIME_HANDLE_TO_FD and
> importing it on a second DRM device with DRM_IOCTL_PRIME_FD_TO_HANDLE
> is sufficient.  Both ioctls are DRM_RENDER_ALLOW.
>
> Observed on 6.12.101 with KASAN, as uid 65534:
>
>   BUG: KASAN: invalid-free in drm_gem_unmap_dma_buf+0xb3/0xf0
>   Free of addr ffff888008290450 by task poc_mm04_sgtabl/321
>   CPU: 1 UID: 65534 PID: 321 Comm: poc_mm04_sgtabl Not tainted 6.12.101 #4
>    kasan_report_invalid_free+0x94/0xc0
>    check_slab_allocation+0x116/0x120
>    kfree+0x103/0x360
>    drm_gem_unmap_dma_buf+0xb3/0xf0
>    dma_buf_detach+0x165/0x510
>    drm_gem_prime_import_dev+0x33e/0x430
>   which belongs to the cache kmalloc-192 of size 192
>   The buggy address is located 80 bytes inside of
>    144-byte region [ffff888008290400, ffff888008290490)
>
> 80 is offsetof(struct vmw_ttm_tt, sgt) and 144 is sizeof(struct
> vmw_ttm_tt), which identifies the freed pointer as the embedded member.
>
> Always return a table the core can own, as the other drivers do.
> Reusing the cached representation would require copying it into a
> freshly allocated sg_table, never returning the alias.
>
> Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> index 39f8c4655..a0233729b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> @@ -73,10 +73,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct drm_gem_object *obj)
>         struct vmw_ttm_tt *vmw_tt =
>                 container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm);
>
> -       if (vmw_tt->vsgt.sgt)
> -               return vmw_tt->vsgt.sgt;
> -
> -       return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages, vmw_tt->dma_ttm.num_pages);
> +       /*
> +        * Do not return &vmw_tt->sgt: the core owns what this returns and
> +        * drm_gem_unmap_dma_buf() sg_free_table()s and kfree()s it, but that
> +        * sg_table is embedded in the vmw_ttm_tt allocation.
> +        */
> +       return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
> +                                    vmw_tt->dma_ttm.num_pages);
>  }
>
>  static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
> --
> 2.43.0
>

Thank you. Because of the influx of llm patches it's very hard to
reason about what's valid and what's not if it comes without a
reproducible testcase. Could you please add an igt testcase for this
and then include the full igt results for vmwgfx before and after this
change? Same for your other change.

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH] drm/vmwgfx: do not hand the embedded sg_table to the PRIME core

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

Hi Aldo,
Using your POC, I am not able to recreate this bug you described in your commit.
This is on a top of tree drm-misc-fixes kernel with kasan enabled.
Are you sure the bug you describe is not fixed on drm-misc-fixes already?

-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH] drm/vmwgfx: do not hand the embedded sg_table to the PRIME core

[Aldo Ariel Panzardo]

Hi Maaz,

Thanks for testing.

I think the reason you could not reproduce is that the original PoC
opens two fds to the same vmwgfx render node.  When the importer
and exporter are the same device, drm_gem_prime_import_dev() hits
the drm_gem_is_prime_exported_dma_buf() check (drm_prime.c:976):
it sees dma_buf->ops == drm_gem_prime_dmabuf_ops && obj->dev == dev,
and returns dma_buf->priv directly without attaching or mapping —
so drm_gem_unmap_dma_buf() is never reached and the kfree never fires.

To hit the invalid-free, the PRIME import must go through a
different DRM device (e.g. virtio-gpu) so the full attachment
map/unmap path is taken.  In addition, the BO must be bound to a GB
surface before the import, so that vmw_ttm_map_dma() runs and
caches vsgt.sgt = &vmw_tt->sgt (the embedded member).  Without the
bind, the TTM is not DMA-mapped and get_sg_table falls through to
drm_prime_pages_to_sg(), which allocates a fresh table — no bug.

With an updated PoC that uses a second GPU for import and binds the
BO first, I was able to reproduce on mainline 7.3-rc4 (6edd14dd67d7):

  BUG: KASAN: invalid-free in drm_gem_unmap_dma_buf+0x6b/0x80
  Free of addr ffff888103c34e60 by task poc_mm04_attack/242

  CPU: 2 UID: 65534 PID: 242 Comm: poc_mm04_attack Not tainted
       7.3.0-rc4-g6edd14dd67d7-dirty #8 PREEMPT(lazy)
  Call Trace:
   <TASK>
   kasan_report_invalid_free+0xb8/0xe0
   check_slab_allocation+0xf5/0x100
   kfree+0x163/0x510
   drm_gem_unmap_dma_buf+0x6b/0x80
   dma_buf_unmap_attachment_unlocked+0x72/0xc0
   drm_gem_prime_import_dev+0x1f5/0x260
   virtgpu_gem_prime_import+0x328/0x580
   drm_gem_prime_fd_to_handle+0x116/0x370
   drm_ioctl+0x3d4/0x790

  Allocated by task 242:
   vmw_ttm_tt_create+0x4c/0x130
   ttm_tt_create+0xca/0x190
   ttm_bo_handle_move_mem+0xea/0x270
   vmw_bo_create+0x248/0x3c0
   vmw_gem_object_create_ioctl+0xa1/0x1a0

  The buggy address belongs to the cache kmalloc-192 of size 192
  The buggy address is located 96 bytes inside of
   160-byte region [ffff888103c34e00, ffff888103c34ea0)

Setup:
  - QEMU with -device vmware-svga -device virtio-gpu-pci
  - Kernel: 7.3-rc4, KASAN, CONFIG_DRM_VMWGFX=y
  - Three local-only changes to make vmwgfx probe on QEMU:
    1. pitchlock error return bypassed (hardware capability
       check, not security-relevant — VMware real passes it)
    2. SVGA_CAP_GBOBJECTS forced (QEMU does not advertise it;
       VMware real does)
    3. max_mob_pages/max_mob_size forced to 256 MB (QEMU
       returns 0; VMware real provides real values)
    None of these touch the PRIME export/import path or the
    sg_table ownership code.
  - DMA map mode: vmw_dma_map_populate (line in boot log:
    "DMA map mode: Caching DMA mappings")

Steps (runs as uid 65534):
  1. DRM_VMW_ALLOC_DMABUF(262144) on vmwgfx renderD128
  2. DRM_IOCTL_PRIME_HANDLE_TO_FD
  3. DRM_VMW_GB_SURFACE_CREATE 256x256
  4. EXECBUF BIND_GB_SURFACE(sid, mobid=handle)
     -> vmw_ttm_bind -> vmw_ttm_map_dma
     -> sets vsgt.sgt = &vmw_tt->sgt  (embedded, vmw_dma_map_populate)
  5. DRM_IOCTL_PRIME_FD_TO_HANDLE on virtio-gpu renderD129
     -> dma_buf_map_attachment -> drm_gem_map_dma_buf
        -> vmw_gem_object_get_sg_table returns &vmw_tt->sgt
     -> virtgpu_gem_prime_import_sg_table fails with -ENODEV
        (no blob resource support in this QEMU config)
     -> error cleanup: dma_buf_unmap_attachment_unlocked
        -> drm_gem_unmap_dma_buf: sg_free_table + kfree(&vmw_tt->sgt)
     => KASAN: invalid-free (interior pointer of vmw_ttm_tt)

I also found a second, related bug in the same function.  When
testing without the GBOBJECTS/MOB patches (i.e. vanilla QEMU where
BOs go to VRAM only), vmw_gem_object_get_sg_table() does
container_of(bo->ttm, ...) without checking that bo->ttm is
non-NULL.  Without MOB, the BO stays in VRAM and no TTM TT is
ever allocated, so bo->ttm is NULL and the dereference faults:

  BUG: KASAN: null-ptr-deref in vmw_gem_object_get_sg_table+0x2f/0xa0
  Read of size 8 at addr 0000000000000088 by task poc_sgtable/330

  CPU: 0 UID: 0 PID: 330 Comm: poc_sgtable Not tainted
       7.3.0-rc4-g6edd14dd67d7-dirty #7 PREEMPT(lazy)
  Call Trace:
   vmw_gem_object_get_sg_table+0x2f/0xa0
   drm_gem_map_dma_buf+0x79/0x120
   dma_buf_map_attachment+0xc1/0x4f0
   drm_gem_prime_import_dev+0xe0/0x260
   virtgpu_gem_prime_import+0x328/0x580
   drm_gem_prime_fd_to_handle+0x116/0x370

  Steps:
    1. DRM_IOCTL_MODE_CREATE_DUMB on vmwgfx (64x64x32)
    2. DRM_IOCTL_PRIME_HANDLE_TO_FD
    3. DRM_IOCTL_PRIME_FD_TO_HANDLE on virtio-gpu
    -> NULL pointer dereference at offset 0x88

I have both PoC sources and the full KASAN/dmesg logs ready.
Regarding Zack's request for igt tests: I have not written those
yet but plan to include them with v2.

Would you like me to send v2 as two patches (1/2 NULL-check,
2/2 embedded sg_table fix) in this same thread, or do you prefer
the NULL-deref fix as a separate submission?

thanks,
Aldo