* [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink @ 2025-12-24 9:40 Barry Song 2025-12-24 17:01 ` Matthew Wilcox 2026-01-05 8:54 ` Baolin Wang 0 siblings, 2 replies; 13+ messages in thread From: Barry Song @ 2025-12-24 9:40 UTC (permalink / raw) To: akpm, linux-mm Cc: linux-kernel, Barry Song, Hugh Dickins, Baolin Wang, syzbot+178fff6149127421c2cc From: Barry Song <baohua@kernel.org> Uninitialized folio allocated in shmem_symlink() may be accessed during swap-out, causing KMSAN BUG: BUG: KMSAN: uninit-value in is_folio_zero_filled mm/page_io.c:188 [inline] BUG: KMSAN: uninit-value in swap_writeout+0x468/0x1390 mm/page_io.c:263 is_folio_zero_filled mm/page_io.c:188 [inline] swap_writeout+0x468/0x1390 mm/page_io.c:263 shmem_writeout+0x1abb/0x1f60 mm/shmem.c:1662 writeout mm/vmscan.c:649 [inline] pageout mm/vmscan.c:698 [inline] shrink_folio_list+0x5920/0x7fc0 mm/vmscan.c:1418 evict_folios+0x999d/0xbf30 mm/vmscan.c:4711 try_to_shrink_lruvec+0x12b6/0x17e0 mm/vmscan.c:4874 lru_gen_shrink_lruvec mm/vmscan.c:5023 [inline] shrink_lruvec+0x46f/0x4f10 mm/vmscan.c:5784 shrink_node_memcgs mm/vmscan.c:6020 [inline] This patch clears the remaining part to zero for the portion not covered by memcpy from symname. Cc: Hugh Dickins <hughd@google.com> Cc: Baolin Wang <baolin.wang@linux.alibaba.com> Reported-by: syzbot+178fff6149127421c2cc@syzkaller.appspotmail.com Closes: https://lore.kernel.org/lkml/6949370f.050a0220.1b4e0c.0038.GAE@google.com/ Signed-off-by: Barry Song <baohua@kernel.org> --- mm/shmem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mm/shmem.c b/mm/shmem.c index ec6c01378e9d..835900a08f51 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -4131,6 +4131,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, goto out_remove_offset; inode->i_op = &shmem_symlink_inode_operations; memcpy(folio_address(folio), symname, len); + folio_zero_range(folio, len, folio_size(folio) - len); folio_mark_uptodate(folio); folio_mark_dirty(folio); folio_unlock(folio); -- 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-24 9:40 [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink Barry Song @ 2025-12-24 17:01 ` Matthew Wilcox 2025-12-25 4:04 ` Barry Song 2026-01-05 8:54 ` Baolin Wang 1 sibling, 1 reply; 13+ messages in thread From: Matthew Wilcox @ 2025-12-24 17:01 UTC (permalink / raw) To: Barry Song Cc: akpm, linux-mm, linux-kernel, Barry Song, Hugh Dickins, Baolin Wang, syzbot+178fff6149127421c2cc On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: > From: Barry Song <baohua@kernel.org> > > Uninitialized folio allocated in shmem_symlink() may be accessed > during swap-out, causing KMSAN BUG: This would be an unfortunate way to fix it. The vast majority of symlinks are short, and we'll never access past the \0 in normal operation, so we'll be dirtying a lot of cachelines essentially to (1) shut up an automated tool and (2) optimise a corner case. How about this instead which delays zeroing to swapout? diff --git a/mm/shmem.c b/mm/shmem.c index ec6c01378e9d..f3b3be1b50fe 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, folio_mark_uptodate(folio); } + /* Zero out symlink tails to help with compression */ + if (folio_test_owner_2(folio)) { + struct inode *inode = folio->mapping->host; + folio_zero_segment(folio, inode->i_size, folio_size(folio)); + folio_clear_owner_2(folio); + } + if (!folio_alloc_swap(folio)) { bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); int error; @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, memcpy(folio_address(folio), symname, len); folio_mark_uptodate(folio); folio_mark_dirty(folio); + folio_set_owner_2(folio); folio_unlock(folio); folio_put(folio); } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-24 17:01 ` Matthew Wilcox @ 2025-12-25 4:04 ` Barry Song 2025-12-25 10:08 ` Baolin Wang 2025-12-28 4:29 ` Matthew Wilcox 0 siblings, 2 replies; 13+ messages in thread From: Barry Song @ 2025-12-25 4:04 UTC (permalink / raw) To: Matthew Wilcox Cc: akpm, linux-mm, linux-kernel, Hugh Dickins, Baolin Wang, syzbot+178fff6149127421c2cc On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: > > From: Barry Song <baohua@kernel.org> > > > > Uninitialized folio allocated in shmem_symlink() may be accessed > > during swap-out, causing KMSAN BUG: > > This would be an unfortunate way to fix it. The vast majority of > symlinks are short, and we'll never access past the \0 in normal > operation, so we'll be dirtying a lot of cachelines essentially to (1) > shut up an automated tool and (2) optimise a corner case. > > How about this instead which delays zeroing to swapout? Matthew, thank you very much for your review, even during Christmas. I would like to wish you a happy holiday! I am not quite sure, as shm symlinks do not seem very common. Since allocating a folio requires a symname longer than 128 bytes (where 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? If so, I am not quite sure it is worth changing the hotpath to accommodate this. > > diff --git a/mm/shmem.c b/mm/shmem.c > index ec6c01378e9d..f3b3be1b50fe 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, > folio_mark_uptodate(folio); > } > > + /* Zero out symlink tails to help with compression */ > + if (folio_test_owner_2(folio)) { > + struct inode *inode = folio->mapping->host; > + folio_zero_segment(folio, inode->i_size, folio_size(folio)); > + folio_clear_owner_2(folio); > + } > + > if (!folio_alloc_swap(folio)) { > bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); > int error; > @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, > memcpy(folio_address(folio), symname, len); > folio_mark_uptodate(folio); > folio_mark_dirty(folio); > + folio_set_owner_2(folio); > folio_unlock(folio); > folio_put(folio); > } Thanks Barry ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-25 4:04 ` Barry Song @ 2025-12-25 10:08 ` Baolin Wang 2026-01-05 13:58 ` Brian Foster 2025-12-28 4:29 ` Matthew Wilcox 1 sibling, 1 reply; 13+ messages in thread From: Baolin Wang @ 2025-12-25 10:08 UTC (permalink / raw) To: Barry Song, Matthew Wilcox Cc: akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On 2025/12/25 12:04, Barry Song wrote: > On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: >> >> On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: >>> From: Barry Song <baohua@kernel.org> >>> >>> Uninitialized folio allocated in shmem_symlink() may be accessed >>> during swap-out, causing KMSAN BUG: >> >> This would be an unfortunate way to fix it. The vast majority of >> symlinks are short, and we'll never access past the \0 in normal >> operation, so we'll be dirtying a lot of cachelines essentially to (1) >> shut up an automated tool and (2) optimise a corner case. >> >> How about this instead which delays zeroing to swapout? > > Matthew, thank you very much for your review, even during Christmas. > I would like to wish you a happy holiday! > > I am not quite sure, as shm symlinks do not seem very common. Since > allocating a folio requires a symname longer than 128 bytes (where > 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. > > BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? > If so, I am not quite sure it is worth changing the hotpath to > accommodate this. +1. At least for me, using the 'PG_owner_2' flag alone to mark this uncommon case doesn't seem quite worthwhile. >> diff --git a/mm/shmem.c b/mm/shmem.c >> index ec6c01378e9d..f3b3be1b50fe 100644 >> --- a/mm/shmem.c >> +++ b/mm/shmem.c >> @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, >> folio_mark_uptodate(folio); >> } >> >> + /* Zero out symlink tails to help with compression */ >> + if (folio_test_owner_2(folio)) { >> + struct inode *inode = folio->mapping->host; >> + folio_zero_segment(folio, inode->i_size, folio_size(folio)); >> + folio_clear_owner_2(folio); >> + } >> + >> if (!folio_alloc_swap(folio)) { >> bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); >> int error; >> @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, >> memcpy(folio_address(folio), symname, len); >> folio_mark_uptodate(folio); >> folio_mark_dirty(folio); >> + folio_set_owner_2(folio); >> folio_unlock(folio); >> folio_put(folio); >> } > > Thanks > Barry ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-25 10:08 ` Baolin Wang @ 2026-01-05 13:58 ` Brian Foster 2026-01-05 17:50 ` Matthew Wilcox 2026-01-06 3:47 ` Baolin Wang 0 siblings, 2 replies; 13+ messages in thread From: Brian Foster @ 2026-01-05 13:58 UTC (permalink / raw) To: Baolin Wang Cc: Barry Song, Matthew Wilcox, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On Thu, Dec 25, 2025 at 06:08:08PM +0800, Baolin Wang wrote: > > > On 2025/12/25 12:04, Barry Song wrote: > > On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: > > > > From: Barry Song <baohua@kernel.org> > > > > > > > > Uninitialized folio allocated in shmem_symlink() may be accessed > > > > during swap-out, causing KMSAN BUG: > > > > > > This would be an unfortunate way to fix it. The vast majority of > > > symlinks are short, and we'll never access past the \0 in normal > > > operation, so we'll be dirtying a lot of cachelines essentially to (1) > > > shut up an automated tool and (2) optimise a corner case. > > > > > > How about this instead which delays zeroing to swapout? > > > > Matthew, thank you very much for your review, even during Christmas. > > I would like to wish you a happy holiday! > > > > I am not quite sure, as shm symlinks do not seem very common. Since > > allocating a folio requires a symname longer than 128 bytes (where > > 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. > > > > BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? > > If so, I am not quite sure it is worth changing the hotpath to > > accommodate this. > > +1. At least for me, using the 'PG_owner_2' flag alone to mark this uncommon > case doesn't seem quite worthwhile. > Also JFYI the post-eof swapout zeroing work (still pending) looks to me like it would cover the swapout time case [1]. That's just if you wanted to go that route here; creation time zeroing for the large symlink case seems reasonable enough to me as well. Brian [1] https://lore.kernel.org/linux-mm/20251121152246.1023918-3-bfoster@redhat.com/ > > > diff --git a/mm/shmem.c b/mm/shmem.c > > > index ec6c01378e9d..f3b3be1b50fe 100644 > > > --- a/mm/shmem.c > > > +++ b/mm/shmem.c > > > @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, > > > folio_mark_uptodate(folio); > > > } > > > > > > + /* Zero out symlink tails to help with compression */ > > > + if (folio_test_owner_2(folio)) { > > > + struct inode *inode = folio->mapping->host; > > > + folio_zero_segment(folio, inode->i_size, folio_size(folio)); > > > + folio_clear_owner_2(folio); > > > + } > > > + > > > if (!folio_alloc_swap(folio)) { > > > bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); > > > int error; > > > @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, > > > memcpy(folio_address(folio), symname, len); > > > folio_mark_uptodate(folio); > > > folio_mark_dirty(folio); > > > + folio_set_owner_2(folio); > > > folio_unlock(folio); > > > folio_put(folio); > > > } > > > > Thanks > > Barry > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-05 13:58 ` Brian Foster @ 2026-01-05 17:50 ` Matthew Wilcox 2026-01-06 3:47 ` Baolin Wang 1 sibling, 0 replies; 13+ messages in thread From: Matthew Wilcox @ 2026-01-05 17:50 UTC (permalink / raw) To: Brian Foster Cc: Baolin Wang, Barry Song, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On Mon, Jan 05, 2026 at 08:58:08AM -0500, Brian Foster wrote: > Also JFYI the post-eof swapout zeroing work (still pending) looks to me > like it would cover the swapout time case [1]. That's just if you wanted > to go that route here; creation time zeroing for the large symlink case > seems reasonable enough to me as well. I think we should merge your patch series instead. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-05 13:58 ` Brian Foster 2026-01-05 17:50 ` Matthew Wilcox @ 2026-01-06 3:47 ` Baolin Wang 2026-01-06 15:56 ` Brian Foster 2026-01-06 18:46 ` Matthew Wilcox 1 sibling, 2 replies; 13+ messages in thread From: Baolin Wang @ 2026-01-06 3:47 UTC (permalink / raw) To: Brian Foster Cc: Barry Song, Matthew Wilcox, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On 1/5/26 9:58 PM, Brian Foster wrote: > On Thu, Dec 25, 2025 at 06:08:08PM +0800, Baolin Wang wrote: >> >> >> On 2025/12/25 12:04, Barry Song wrote: >>> On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: >>>> >>>> On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: >>>>> From: Barry Song <baohua@kernel.org> >>>>> >>>>> Uninitialized folio allocated in shmem_symlink() may be accessed >>>>> during swap-out, causing KMSAN BUG: >>>> >>>> This would be an unfortunate way to fix it. The vast majority of >>>> symlinks are short, and we'll never access past the \0 in normal >>>> operation, so we'll be dirtying a lot of cachelines essentially to (1) >>>> shut up an automated tool and (2) optimise a corner case. >>>> >>>> How about this instead which delays zeroing to swapout? >>> >>> Matthew, thank you very much for your review, even during Christmas. >>> I would like to wish you a happy holiday! >>> >>> I am not quite sure, as shm symlinks do not seem very common. Since >>> allocating a folio requires a symname longer than 128 bytes (where >>> 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. >>> >>> BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? >>> If so, I am not quite sure it is worth changing the hotpath to >>> accommodate this. >> >> +1. At least for me, using the 'PG_owner_2' flag alone to mark this uncommon >> case doesn't seem quite worthwhile. >> > > Also JFYI the post-eof swapout zeroing work (still pending) looks to me > like it would cover the swapout time case [1]. That's just if you wanted > to go that route here; creation time zeroing for the large symlink case > seems reasonable enough to me as well. IMHO, your post-eof zeroing work is intended to zero !uptodate or beyond EOF folios before swap-out, however, the symlink folio is uptodate and not beyond the EOF. I am not sure if it will make your code more complicated when you want to cover this symlink case. Why I prefer Barry's fix: First, the symlink folio is marked Uptodate after copying the symlink name, but the whole folio hasn’t been initialized, which seems unreasonable to me. Second, as I said before, using the 'PG_owner_2' flag to mark this uncommon case doesn’t seem worthwhile. Currently, IIUC the 'PG_owner_2' is only used by btrfs; if we ever want to remove the 'PG_owner_2', this uncommon symlink case shouldn’t block its removal. BTW, as I said before, I've reviewed and tested your post-eof work[1]. Maybe you could resend the patch set so that Hugh can take a look when he has time. > [1] https://lore.kernel.org/linux-mm/20251121152246.1023918-3-bfoster@redhat.com/ > >>>> diff --git a/mm/shmem.c b/mm/shmem.c >>>> index ec6c01378e9d..f3b3be1b50fe 100644 >>>> --- a/mm/shmem.c >>>> +++ b/mm/shmem.c >>>> @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, >>>> folio_mark_uptodate(folio); >>>> } >>>> >>>> + /* Zero out symlink tails to help with compression */ >>>> + if (folio_test_owner_2(folio)) { >>>> + struct inode *inode = folio->mapping->host; >>>> + folio_zero_segment(folio, inode->i_size, folio_size(folio)); >>>> + folio_clear_owner_2(folio); >>>> + } >>>> + >>>> if (!folio_alloc_swap(folio)) { >>>> bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); >>>> int error; >>>> @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, >>>> memcpy(folio_address(folio), symname, len); >>>> folio_mark_uptodate(folio); >>>> folio_mark_dirty(folio); >>>> + folio_set_owner_2(folio); >>>> folio_unlock(folio); >>>> folio_put(folio); >>>> } >>> >>> Thanks >>> Barry >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-06 3:47 ` Baolin Wang @ 2026-01-06 15:56 ` Brian Foster 2026-01-07 1:12 ` Baolin Wang 2026-01-06 18:46 ` Matthew Wilcox 1 sibling, 1 reply; 13+ messages in thread From: Brian Foster @ 2026-01-06 15:56 UTC (permalink / raw) To: Baolin Wang Cc: Barry Song, Matthew Wilcox, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On Tue, Jan 06, 2026 at 11:47:44AM +0800, Baolin Wang wrote: > > > On 1/5/26 9:58 PM, Brian Foster wrote: > > On Thu, Dec 25, 2025 at 06:08:08PM +0800, Baolin Wang wrote: > > > > > > > > > On 2025/12/25 12:04, Barry Song wrote: > > > > On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > > > On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: > > > > > > From: Barry Song <baohua@kernel.org> > > > > > > > > > > > > Uninitialized folio allocated in shmem_symlink() may be accessed > > > > > > during swap-out, causing KMSAN BUG: > > > > > > > > > > This would be an unfortunate way to fix it. The vast majority of > > > > > symlinks are short, and we'll never access past the \0 in normal > > > > > operation, so we'll be dirtying a lot of cachelines essentially to (1) > > > > > shut up an automated tool and (2) optimise a corner case. > > > > > > > > > > How about this instead which delays zeroing to swapout? > > > > > > > > Matthew, thank you very much for your review, even during Christmas. > > > > I would like to wish you a happy holiday! > > > > > > > > I am not quite sure, as shm symlinks do not seem very common. Since > > > > allocating a folio requires a symname longer than 128 bytes (where > > > > 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. > > > > > > > > BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? > > > > If so, I am not quite sure it is worth changing the hotpath to > > > > accommodate this. > > > > > > +1. At least for me, using the 'PG_owner_2' flag alone to mark this uncommon > > > case doesn't seem quite worthwhile. > > > > > > > Also JFYI the post-eof swapout zeroing work (still pending) looks to me > > like it would cover the swapout time case [1]. That's just if you wanted > > to go that route here; creation time zeroing for the large symlink case > > seems reasonable enough to me as well. > > IMHO, your post-eof zeroing work is intended to zero !uptodate or beyond EOF > folios before swap-out, however, the symlink folio is uptodate and not > beyond the EOF. I am not sure if it will make your code more complicated > when you want to cover this symlink case. > It already handles this particular case. The primary intent is to zero all post-eof ranges that can be written out before that write out happens. I.e., if you take a look at shmem_writeout(), if the folio happens to straddle i_size we zero from i_size to the end of the folio. > Why I prefer Barry's fix: First, the symlink folio is marked Uptodate after > copying the symlink name, but the whole folio hasn’t been initialized, which > seems unreasonable to me. Second, as I said before, using the 'PG_owner_2' > flag to mark this uncommon case doesn’t seem worthwhile. Currently, IIUC the > 'PG_owner_2' is only used by btrfs; if we ever want to remove the > 'PG_owner_2', this uncommon symlink case shouldn’t block its removal. > I think we're talking past eachother a bit here. ;) I'm pointing out that outstanding work already covers writeout time zeroing so as to avoid duplicate effort if this was trending that way. IOW, I think there's no reason to consider the page flag thing, it's more of a question of do you want the zeroing at creation time or not.. I again don't have a strong opinion on this, but I think I can see justification either way. If the focus is a KMSAN splat at writeout time, then it makes some sense to address it in that slower/less likely path. That said, if we step back and consider that if this were a buffered write we'd tail zero the folio at write time before marking it uptodate, this seems like more broadly consistent behavior with other fs' and operations. This would be analogous to say XFS allocating a zeroed metadata buffer on symlink creation to ensure the underlying block is tail zeroed before it is eventually written to disk. So to your point, maybe the most appropriate thing to do is cover zeroing in both places.. > BTW, as I said before, I've reviewed and tested your post-eof work[1]. Maybe > you could resend the patch set so that Hugh can take a look when he has > time. > Yeah.. I know Hugh is busy and that isn't the highest priority series. I'm just catching back up from time off myself so I can give it more time before I repost it or anything. Brian > > [1] https://lore.kernel.org/linux-mm/20251121152246.1023918-3-bfoster@redhat.com/ > > > > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > > > > index ec6c01378e9d..f3b3be1b50fe 100644 > > > > > --- a/mm/shmem.c > > > > > +++ b/mm/shmem.c > > > > > @@ -1636,6 +1636,13 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug, > > > > > folio_mark_uptodate(folio); > > > > > } > > > > > > > > > > + /* Zero out symlink tails to help with compression */ > > > > > + if (folio_test_owner_2(folio)) { > > > > > + struct inode *inode = folio->mapping->host; > > > > > + folio_zero_segment(folio, inode->i_size, folio_size(folio)); > > > > > + folio_clear_owner_2(folio); > > > > > + } > > > > > + > > > > > if (!folio_alloc_swap(folio)) { > > > > > bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages); > > > > > int error; > > > > > @@ -4133,6 +4140,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, > > > > > memcpy(folio_address(folio), symname, len); > > > > > folio_mark_uptodate(folio); > > > > > folio_mark_dirty(folio); > > > > > + folio_set_owner_2(folio); > > > > > folio_unlock(folio); > > > > > folio_put(folio); > > > > > } > > > > > > > > Thanks > > > > Barry > > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-06 15:56 ` Brian Foster @ 2026-01-07 1:12 ` Baolin Wang 0 siblings, 0 replies; 13+ messages in thread From: Baolin Wang @ 2026-01-07 1:12 UTC (permalink / raw) To: Brian Foster Cc: Barry Song, Matthew Wilcox, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On 1/6/26 11:56 PM, Brian Foster wrote: > On Tue, Jan 06, 2026 at 11:47:44AM +0800, Baolin Wang wrote: >> >> >> On 1/5/26 9:58 PM, Brian Foster wrote: >>> On Thu, Dec 25, 2025 at 06:08:08PM +0800, Baolin Wang wrote: >>>> >>>> >>>> On 2025/12/25 12:04, Barry Song wrote: >>>>> On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: >>>>>> >>>>>> On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: >>>>>>> From: Barry Song <baohua@kernel.org> >>>>>>> >>>>>>> Uninitialized folio allocated in shmem_symlink() may be accessed >>>>>>> during swap-out, causing KMSAN BUG: >>>>>> >>>>>> This would be an unfortunate way to fix it. The vast majority of >>>>>> symlinks are short, and we'll never access past the \0 in normal >>>>>> operation, so we'll be dirtying a lot of cachelines essentially to (1) >>>>>> shut up an automated tool and (2) optimise a corner case. >>>>>> >>>>>> How about this instead which delays zeroing to swapout? >>>>> >>>>> Matthew, thank you very much for your review, even during Christmas. >>>>> I would like to wish you a happy holiday! >>>>> >>>>> I am not quite sure, as shm symlinks do not seem very common. Since >>>>> allocating a folio requires a symname longer than 128 bytes (where >>>>> 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. >>>>> >>>>> BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? >>>>> If so, I am not quite sure it is worth changing the hotpath to >>>>> accommodate this. >>>> >>>> +1. At least for me, using the 'PG_owner_2' flag alone to mark this uncommon >>>> case doesn't seem quite worthwhile. >>>> >>> >>> Also JFYI the post-eof swapout zeroing work (still pending) looks to me >>> like it would cover the swapout time case [1]. That's just if you wanted >>> to go that route here; creation time zeroing for the large symlink case >>> seems reasonable enough to me as well. >> >> IMHO, your post-eof zeroing work is intended to zero !uptodate or beyond EOF >> folios before swap-out, however, the symlink folio is uptodate and not >> beyond the EOF. I am not sure if it will make your code more complicated >> when you want to cover this symlink case. >> > > It already handles this particular case. The primary intent is to zero > all post-eof ranges that can be written out before that write out > happens. I.e., if you take a look at shmem_writeout(), if the folio > happens to straddle i_size we zero from i_size to the end of the folio. Ah, yes. I’ve rechecked your patch set, and you’re right. >> Why I prefer Barry's fix: First, the symlink folio is marked Uptodate after >> copying the symlink name, but the whole folio hasn’t been initialized, which >> seems unreasonable to me. Second, as I said before, using the 'PG_owner_2' >> flag to mark this uncommon case doesn’t seem worthwhile. Currently, IIUC the >> 'PG_owner_2' is only used by btrfs; if we ever want to remove the >> 'PG_owner_2', this uncommon symlink case shouldn’t block its removal. >> > > I think we're talking past eachother a bit here. ;) I'm pointing out > that outstanding work already covers writeout time zeroing so as to > avoid duplicate effort if this was trending that way. IOW, I think > there's no reason to consider the page flag thing, it's more of a > question of do you want the zeroing at creation time or not.. Yes. It is very strange that a folio is marked uptodate but is not fully initialized. > I again don't have a strong opinion on this, but I think I can see > justification either way. If the focus is a KMSAN splat at writeout > time, then it makes some sense to address it in that slower/less likely > path. > > That said, if we step back and consider that if this were a buffered > write we'd tail zero the folio at write time before marking it uptodate, > this seems like more broadly consistent behavior with other fs' and > operations. This would be analogous to say XFS allocating a zeroed > metadata buffer on symlink creation to ensure the underlying block is > tail zeroed before it is eventually written to disk. > > So to your point, maybe the most appropriate thing to do is cover > zeroing in both places.. Agree. >> BTW, as I said before, I've reviewed and tested your post-eof work[1]. Maybe >> you could resend the patch set so that Hugh can take a look when he has >> time. >> > > Yeah.. I know Hugh is busy and that isn't the highest priority series. > I'm just catching back up from time off myself so I can give it more > time before I repost it or anything. Sure. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-06 3:47 ` Baolin Wang 2026-01-06 15:56 ` Brian Foster @ 2026-01-06 18:46 ` Matthew Wilcox 2026-01-07 1:16 ` Baolin Wang 1 sibling, 1 reply; 13+ messages in thread From: Matthew Wilcox @ 2026-01-06 18:46 UTC (permalink / raw) To: Baolin Wang Cc: Brian Foster, Barry Song, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On Tue, Jan 06, 2026 at 11:47:44AM +0800, Baolin Wang wrote: > Why I prefer Barry's fix: First, the symlink folio is marked Uptodate after > copying the symlink name, but the whole folio hasn’t been initialized, which > seems unreasonable to me. Second, as I said before, using the 'PG_owner_2' > flag to mark this uncommon case doesn’t seem worthwhile. Currently, IIUC the > 'PG_owner_2' is only used by btrfs; if we ever want to remove the > 'PG_owner_2', this uncommon symlink case shouldn’t block its removal. PG_owner_2 is aliased with PG_mappedtodisk [1], so it's used by every filesystem which uses buffer_heads (whether mentioned in that filesystem or not). btrfs was switched from using private_2 to using owner_2 a little over a year ago. PG_owner_2 is not on the list of flags to be removed; that's PG_private, PG_private_2 and PG_reserved. [1] include/linux/page-flags.h: PG_mappedtodisk = PG_owner_2, ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2026-01-06 18:46 ` Matthew Wilcox @ 2026-01-07 1:16 ` Baolin Wang 0 siblings, 0 replies; 13+ messages in thread From: Baolin Wang @ 2026-01-07 1:16 UTC (permalink / raw) To: Matthew Wilcox Cc: Brian Foster, Barry Song, akpm, linux-mm, linux-kernel, Hugh Dickins, syzbot+178fff6149127421c2cc On 1/7/26 2:46 AM, Matthew Wilcox wrote: > On Tue, Jan 06, 2026 at 11:47:44AM +0800, Baolin Wang wrote: >> Why I prefer Barry's fix: First, the symlink folio is marked Uptodate after >> copying the symlink name, but the whole folio hasn’t been initialized, which >> seems unreasonable to me. Second, as I said before, using the 'PG_owner_2' >> flag to mark this uncommon case doesn’t seem worthwhile. Currently, IIUC the >> 'PG_owner_2' is only used by btrfs; if we ever want to remove the >> 'PG_owner_2', this uncommon symlink case shouldn’t block its removal. > > PG_owner_2 is aliased with PG_mappedtodisk [1], so it's used by every > filesystem which uses buffer_heads (whether mentioned in that filesystem > or not). btrfs was switched from using private_2 to using owner_2 > a little over a year ago. PG_owner_2 is not on the list of flags to be > removed; that's PG_private, PG_private_2 and PG_reserved. OK. Thanks for the explanation. I understand the plan. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-25 4:04 ` Barry Song 2025-12-25 10:08 ` Baolin Wang @ 2025-12-28 4:29 ` Matthew Wilcox 1 sibling, 0 replies; 13+ messages in thread From: Matthew Wilcox @ 2025-12-28 4:29 UTC (permalink / raw) To: Barry Song Cc: akpm, linux-mm, linux-kernel, Hugh Dickins, Baolin Wang, syzbot+178fff6149127421c2cc On Thu, Dec 25, 2025 at 05:04:38PM +1300, Barry Song wrote: > On Thu, Dec 25, 2025 at 6:01 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Wed, Dec 24, 2025 at 10:40:27PM +1300, Barry Song wrote: > > > From: Barry Song <baohua@kernel.org> > > > > > > Uninitialized folio allocated in shmem_symlink() may be accessed > > > during swap-out, causing KMSAN BUG: > > > > This would be an unfortunate way to fix it. The vast majority of > > symlinks are short, and we'll never access past the \0 in normal > > operation, so we'll be dirtying a lot of cachelines essentially to (1) > > shut up an automated tool and (2) optimise a corner case. > > > > How about this instead which delays zeroing to swapout? > > Matthew, thank you very much for your review, even during Christmas. > I would like to wish you a happy holiday! Heh, thanks. My mailserver is actually in a different timezone from me, so it was still the 24th when I sent it ;-) > I am not quite sure, as shm symlinks do not seem very common. Since > allocating a folio requires a symname longer than 128 bytes (where > 128 == SHORT_SYMLINK_LEN), such cases appear even rarer. Fair enough. I hadn't noticed the SHORT_SYMLINK_LEN case when I wrote this. That does change things somewhat, but still for a 160 byte symlink, we have 5 cachelines of data and 59 cachelines of zeroes. > BTW, do we need to migrate the owner_2 flag in folio_migrate_flags()? > If so, I am not quite sure it is worth changing the hotpath to > accommodate this. It already happens, it's just camouflaged. owner_2 is the same bit as mappedtodisk. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink 2025-12-24 9:40 [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink Barry Song 2025-12-24 17:01 ` Matthew Wilcox @ 2026-01-05 8:54 ` Baolin Wang 1 sibling, 0 replies; 13+ messages in thread From: Baolin Wang @ 2026-01-05 8:54 UTC (permalink / raw) To: Barry Song, akpm, linux-mm Cc: linux-kernel, Barry Song, Hugh Dickins, syzbot+178fff6149127421c2cc On 12/24/25 5:40 PM, Barry Song wrote: > From: Barry Song <baohua@kernel.org> > > Uninitialized folio allocated in shmem_symlink() may be accessed > during swap-out, causing KMSAN BUG: > > BUG: KMSAN: uninit-value in is_folio_zero_filled mm/page_io.c:188 [inline] > BUG: KMSAN: uninit-value in swap_writeout+0x468/0x1390 mm/page_io.c:263 > is_folio_zero_filled mm/page_io.c:188 [inline] > swap_writeout+0x468/0x1390 mm/page_io.c:263 > shmem_writeout+0x1abb/0x1f60 mm/shmem.c:1662 > writeout mm/vmscan.c:649 [inline] > pageout mm/vmscan.c:698 [inline] > shrink_folio_list+0x5920/0x7fc0 mm/vmscan.c:1418 > evict_folios+0x999d/0xbf30 mm/vmscan.c:4711 > try_to_shrink_lruvec+0x12b6/0x17e0 mm/vmscan.c:4874 > lru_gen_shrink_lruvec mm/vmscan.c:5023 [inline] > shrink_lruvec+0x46f/0x4f10 mm/vmscan.c:5784 > shrink_node_memcgs mm/vmscan.c:6020 [inline] > > This patch clears the remaining part to zero for the portion not > covered by memcpy from symname. > > Cc: Hugh Dickins <hughd@google.com> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com> > Reported-by: syzbot+178fff6149127421c2cc@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/lkml/6949370f.050a0220.1b4e0c.0038.GAE@google.com/ > Signed-off-by: Barry Song <baohua@kernel.org> > --- LGTM. Thanks. Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> > mm/shmem.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/shmem.c b/mm/shmem.c > index ec6c01378e9d..835900a08f51 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -4131,6 +4131,7 @@ static int shmem_symlink(struct mnt_idmap *idmap, struct inode *dir, > goto out_remove_offset; > inode->i_op = &shmem_symlink_inode_operations; > memcpy(folio_address(folio), symname, len); > + folio_zero_range(folio, len, folio_size(folio) - len); > folio_mark_uptodate(folio); > folio_mark_dirty(folio); > folio_unlock(folio); ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-01-07 1:16 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-24 9:40 [PATCH] mm/shmem: fix uninitialized folio in shmem_symlink Barry Song 2025-12-24 17:01 ` Matthew Wilcox 2025-12-25 4:04 ` Barry Song 2025-12-25 10:08 ` Baolin Wang 2026-01-05 13:58 ` Brian Foster 2026-01-05 17:50 ` Matthew Wilcox 2026-01-06 3:47 ` Baolin Wang 2026-01-06 15:56 ` Brian Foster 2026-01-07 1:12 ` Baolin Wang 2026-01-06 18:46 ` Matthew Wilcox 2026-01-07 1:16 ` Baolin Wang 2025-12-28 4:29 ` Matthew Wilcox 2026-01-05 8:54 ` Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox