* [PATCH] drm/panthor: set __GFP_SKIP_KASAN
@ 2026-05-12 17:36 Chia-I Wu via B4 Relay
2026-05-13 9:11 ` Boris Brezillon
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Chia-I Wu via B4 Relay @ 2026-05-12 17:36 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Rob Clark, Chia-I Wu
From: Chia-I Wu <olvaffe@gmail.com>
Pages that can be swapped out should be allocated with __GFP_SKIP_KASAN.
Rather than setting the flag directly, replace GFP_HIGHUSER by
(GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) instead, which should match the
preceding comment better.
On a CONFIG_KASAN_HW_TAGS=y system, without __GFP_SKIP_KASAN, the page
allocator assigns a valid tag to both the kernel mapping and MTE,
instead of assigning the match-all KASAN_TAG_KERNEL tag to the kernel
mapping. If userspace also maps the page with PROT_MTE and modifies the
MTE tag, accessing the page via the kernel mapping results in KASAN
invalid-access, such as
BUG: KASAN: invalid-access in swap_writepage+0xb0/0x21c
Read at addr f5ffff81aa71dff8 by task WM.task-4/6956
Pointer tag: [f5], memory tag: [f9]
While userspace cannot map drm gem objects with PROT_MTE, the problem is
shmem_swapin_cluster. When it swaps in a cluster of pages using our gfp
flags, some of the pages might belong to other mappings that have
PROT_MTE.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
---
The latest snapdragons appear to have MTE support. drm/msm might need
the same treatment.
---
drivers/gpu/drm/panthor/panthor_gem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index 13295d7a593df..08c03aa0db2f7 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -1013,7 +1013,8 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
* going to pin these pages.
*/
mapping_set_gfp_mask(bo->base.filp->f_mapping,
- GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
+ (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) |
+ __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
ret = drm_gem_create_mmap_offset(&bo->base);
if (ret)
---
base-commit: 6101f78b684895d5860a96322e607e0f46f433ad
change-id: 20260512-panthor-kasan-10477239bad1
Best regards,
--
Chia-I Wu <olvaffe@gmail.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/panthor: set __GFP_SKIP_KASAN
2026-05-12 17:36 [PATCH] drm/panthor: set __GFP_SKIP_KASAN Chia-I Wu via B4 Relay
@ 2026-05-13 9:11 ` Boris Brezillon
2026-05-13 18:39 ` Chia-I Wu
2026-05-16 3:08 ` Claude review: " Claude Code Review Bot
2026-05-16 3:08 ` Claude Code Review Bot
2 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2026-05-13 9:11 UTC (permalink / raw)
To: Chia-I Wu via B4 Relay
Cc: olvaffe, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
dri-devel, linux-kernel, Rob Clark
On Tue, 12 May 2026 10:36:28 -0700
Chia-I Wu via B4 Relay <devnull+olvaffe.gmail.com@kernel.org> wrote:
> From: Chia-I Wu <olvaffe@gmail.com>
>
> Pages that can be swapped out should be allocated with __GFP_SKIP_KASAN.
> Rather than setting the flag directly, replace GFP_HIGHUSER by
> (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) instead, which should match the
> preceding comment better.
I'm not too sure GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE is clearer than
GFP_HIGHUSER (without MOVABLE in the name) to reflect we don't want the
MOVABLE property.
>
> On a CONFIG_KASAN_HW_TAGS=y system, without __GFP_SKIP_KASAN, the page
> allocator assigns a valid tag to both the kernel mapping and MTE,
> instead of assigning the match-all KASAN_TAG_KERNEL tag to the kernel
> mapping. If userspace also maps the page with PROT_MTE and modifies the
> MTE tag, accessing the page via the kernel mapping results in KASAN
> invalid-access, such as
>
> BUG: KASAN: invalid-access in swap_writepage+0xb0/0x21c
> Read at addr f5ffff81aa71dff8 by task WM.task-4/6956
> Pointer tag: [f5], memory tag: [f9]
>
> While userspace cannot map drm gem objects with PROT_MTE, the problem is
> shmem_swapin_cluster. When it swaps in a cluster of pages using our gfp
> flags, some of the pages might belong to other mappings that have
> PROT_MTE.
Okay, let me see if I get this right. The gfp flags we set right now
do what we want for out GEM mappings, it's only when swap readahead is
involved that this becomes problematic:
1. swapin folio for GEM inode X
2. shmem layer optimistically does a readahead on the global swap, with
our GEM gfp flags
3. the next swap entry belongs to some other tmpfs/anonymous mapping
(not even something on the same GEM mountpoint actually)
4. because it's our gfp flags that get used for the folio allocation,
the KASAN poisoning takes place, thus messing up with the original
user-request PROT_MTE
To me, this looks like a bad isolation of the readahead logic: we
shouldn't cross the inode boundary, because gfp flags for one inode
might be completely different from gfp flags for a different inode.
Let alone the fact the boundary being crossed here is not just inode,
it's basically the entire tmpfs mountpoint.
TLDR; I strongly believe we're papering over some more fundamental
issue in the shmem swap readahead logic: with MTE, we can no longer
assume gfp flags are interchangeable, they have to be enforced and
match exactly what the user of the swap entry expects, meaning
readahead should be constrained by the inode boundary.
>
> Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
> ---
> The latest snapdragons appear to have MTE support. drm/msm might need
> the same treatment.
> ---
> drivers/gpu/drm/panthor/panthor_gem.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 13295d7a593df..08c03aa0db2f7 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -1013,7 +1013,8 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> * going to pin these pages.
> */
> mapping_set_gfp_mask(bo->base.filp->f_mapping,
> - GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) |
> + __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
>
> ret = drm_gem_create_mmap_offset(&bo->base);
> if (ret)
>
> ---
> base-commit: 6101f78b684895d5860a96322e607e0f46f433ad
> change-id: 20260512-panthor-kasan-10477239bad1
>
> Best regards,
> --
> Chia-I Wu <olvaffe@gmail.com>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/panthor: set __GFP_SKIP_KASAN
2026-05-13 9:11 ` Boris Brezillon
@ 2026-05-13 18:39 ` Chia-I Wu
0 siblings, 0 replies; 5+ messages in thread
From: Chia-I Wu @ 2026-05-13 18:39 UTC (permalink / raw)
To: Boris Brezillon
Cc: Chia-I Wu via B4 Relay, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel, Rob Clark,
Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
Dmitry Vyukov, Vincenzo Frascino, Andrew Morton, Hugh Dickins,
Baolin Wang, kasan-dev, linux-mm
+KASAN and shmem maintainers
On Wed, May 13, 2026 at 2:11 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Tue, 12 May 2026 10:36:28 -0700
> Chia-I Wu via B4 Relay <devnull+olvaffe.gmail.com@kernel.org> wrote:
>
> > From: Chia-I Wu <olvaffe@gmail.com>
> >
> > Pages that can be swapped out should be allocated with __GFP_SKIP_KASAN.
> > Rather than setting the flag directly, replace GFP_HIGHUSER by
> > (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) instead, which should match the
> > preceding comment better.
>
> I'm not too sure GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE is clearer than
> GFP_HIGHUSER (without MOVABLE in the name) to reflect we don't want the
> MOVABLE property.
>
> >
> > On a CONFIG_KASAN_HW_TAGS=y system, without __GFP_SKIP_KASAN, the page
> > allocator assigns a valid tag to both the kernel mapping and MTE,
> > instead of assigning the match-all KASAN_TAG_KERNEL tag to the kernel
> > mapping. If userspace also maps the page with PROT_MTE and modifies the
> > MTE tag, accessing the page via the kernel mapping results in KASAN
> > invalid-access, such as
> >
> > BUG: KASAN: invalid-access in swap_writepage+0xb0/0x21c
> > Read at addr f5ffff81aa71dff8 by task WM.task-4/6956
> > Pointer tag: [f5], memory tag: [f9]
> >
> > While userspace cannot map drm gem objects with PROT_MTE, the problem is
> > shmem_swapin_cluster. When it swaps in a cluster of pages using our gfp
> > flags, some of the pages might belong to other mappings that have
> > PROT_MTE.
>
> Okay, let me see if I get this right. The gfp flags we set right now
> do what we want for out GEM mappings, it's only when swap readahead is
> involved that this becomes problematic:
>
> 1. swapin folio for GEM inode X
> 2. shmem layer optimistically does a readahead on the global swap, with
> our GEM gfp flags
> 3. the next swap entry belongs to some other tmpfs/anonymous mapping
> (not even something on the same GEM mountpoint actually)
> 4. because it's our gfp flags that get used for the folio allocation,
> the KASAN poisoning takes place, thus messing up with the original
> user-request PROT_MTE
Yes, that's right. At least that's what I observed.
A little more on the last point. shmem readahead allocates a folio for
an unrelated mapping using our gfp flags. It unpoisons both the page
tag and mte tag for the folio because our gfp flags do not have
__GFP_SKIP_KASAN.
When the unrelated mapping is an anonymous one, and when do_swap_page
maps the folio, arch_swap_restore restores the mte tag. This creates
mismatching page tag and mte tag. When, for example, swap_writepage
tries to access the folio, kasan triggers a false positive error.
>
> To me, this looks like a bad isolation of the readahead logic: we
> shouldn't cross the inode boundary, because gfp flags for one inode
> might be completely different from gfp flags for a different inode.
> Let alone the fact the boundary being crossed here is not just inode,
> it's basically the entire tmpfs mountpoint.
>
> TLDR; I strongly believe we're papering over some more fundamental
> issue in the shmem swap readahead logic: with MTE, we can no longer
> assume gfp flags are interchangeable, they have to be enforced and
> match exactly what the user of the swap entry expects, meaning
> readahead should be constrained by the inode boundary.
I totally agree. shmem should not readahead past its inode. Or perhaps
it should force __GFP_SKIP_KASAN when it does.
Adding shmem and kasan maintainers for suggestions/corrections.
>
> >
> > Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
> > ---
> > The latest snapdragons appear to have MTE support. drm/msm might need
> > the same treatment.
> > ---
> > drivers/gpu/drm/panthor/panthor_gem.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> > index 13295d7a593df..08c03aa0db2f7 100644
> > --- a/drivers/gpu/drm/panthor/panthor_gem.c
> > +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> > @@ -1013,7 +1013,8 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> > * going to pin these pages.
> > */
> > mapping_set_gfp_mask(bo->base.filp->f_mapping,
> > - GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> > + (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) |
> > + __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> >
> > ret = drm_gem_create_mmap_offset(&bo->base);
> > if (ret)
> >
> > ---
> > base-commit: 6101f78b684895d5860a96322e607e0f46f433ad
> > change-id: 20260512-panthor-kasan-10477239bad1
> >
> > Best regards,
> > --
> > Chia-I Wu <olvaffe@gmail.com>
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/panthor: set __GFP_SKIP_KASAN
2026-05-12 17:36 [PATCH] drm/panthor: set __GFP_SKIP_KASAN Chia-I Wu via B4 Relay
2026-05-13 9:11 ` Boris Brezillon
@ 2026-05-16 3:08 ` Claude Code Review Bot
2026-05-16 3:08 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:08 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/panthor: set __GFP_SKIP_KASAN
Author: Chia-I Wu via B4 Relay <devnull+olvaffe.gmail.com@kernel.org>
Patches: 3
Reviewed: 2026-05-16T13:08:46.783545
---
This is a single, well-motivated patch that fixes a real KASAN invalid-access bug on CONFIG_KASAN_HW_TAGS=y systems with MTE-capable hardware (recent Snapdragon SoCs). The problem is clearly explained: `shmem_swapin_cluster` can swap in pages belonging to other PROT_MTE mappings using panthor's GFP flags, and without `__GFP_SKIP_KASAN`, the page allocator assigns a specific KASAN tag to the kernel mapping instead of the match-all tag, leading to tag mismatch crashes.
The fix is correct. The approach of deriving the flags from `GFP_HIGHUSER_MOVABLE` rather than directly adding `__GFP_SKIP_KASAN` to the existing expression is a reasonable choice that better matches the semantic intent — these are user-facing pages that should behave like `GFP_HIGHUSER_MOVABLE` except they must not be movable (because panthor pins them).
**Verdict: The patch looks correct and ready for merging**, with minor style observations below.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/panthor: set __GFP_SKIP_KASAN
2026-05-12 17:36 [PATCH] drm/panthor: set __GFP_SKIP_KASAN Chia-I Wu via B4 Relay
2026-05-13 9:11 ` Boris Brezillon
2026-05-16 3:08 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 3:08 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:08 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness analysis:**
The key expansion check:
- `GFP_HIGHUSER` = `GFP_USER | __GFP_HIGHMEM`
- `GFP_HIGHUSER_MOVABLE` = `GFP_HIGHUSER | __GFP_MOVABLE | __GFP_SKIP_KASAN`
- `GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE` = `GFP_HIGHUSER | __GFP_SKIP_KASAN`
So the new expression:
```c
(GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
```
is equivalent to the old:
```c
GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
```
plus `__GFP_SKIP_KASAN`. This is exactly the intended change — the only new flag added is `__GFP_SKIP_KASAN`.
**Concerns:**
1. **Fragility of the indirection:** The expression `(GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE)` is a somewhat indirect way to spell `GFP_HIGHUSER | __GFP_SKIP_KASAN`. If `GFP_HIGHUSER_MOVABLE` ever gains additional flags beyond `__GFP_MOVABLE | __GFP_SKIP_KASAN`, those would silently be included here. However, this is a common kernel pattern (e.g., `GFP_TRANSHUGE_LIGHT` does a similar mask-and-strip), and the commit message explains the reasoning, so this is acceptable.
2. **Alternative: using `__GFP_SKIP_KASAN` directly.** A more explicit alternative would be:
```c
GFP_HIGHUSER | __GFP_SKIP_KASAN | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
```
This is arguably clearer about what's happening, but the author's approach has the advantage of tying the flags to `GFP_HIGHUSER_MOVABLE` semantics (these pages *would* be movable if panthor didn't pin them), which is a reasonable design choice and matches the existing comment about why `MOVABLE` is stripped.
3. **Alignment nit:** The `__GFP_RETRY_MAYFAIL` on the continuation line is indented with tabs plus extra spaces to align with the opening `(`, but the alignment looks like it might use one extra tab. This is very minor and depends on the project's tab-stop convention.
4. **The `drm/msm` note** in the commit's post-sign-off section is a useful heads-up. If msm GEM objects can also be swapped via shmem and the system has MTE, the same bug would apply there. This is worth tracking but doesn't affect this patch.
**Overall for this patch:** Correct fix for a real bug. The commit message is excellent — it explains the root cause clearly, including the non-obvious `shmem_swapin_cluster` interaction path. No functional issues found.
Reviewed-by: would be appropriate here.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-16 3:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 17:36 [PATCH] drm/panthor: set __GFP_SKIP_KASAN Chia-I Wu via B4 Relay
2026-05-13 9:11 ` Boris Brezillon
2026-05-13 18:39 ` Chia-I Wu
2026-05-16 3:08 ` Claude review: " Claude Code Review Bot
2026-05-16 3:08 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox