All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping()
@ 2026-07-13 11:57 Daniel Palmer
  2026-07-13 19:49 ` Hajime Tazaki
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Palmer @ 2026-07-13 11:57 UTC (permalink / raw)
  To: ljs, akpm, dhowells; +Cc: linux-kernel, thehajime, Daniel Palmer, Sashiko

ramfs_nommu_expand_for_mapping() sets the new i_size before it has
allocated or inserted any of the contiguous backing pages.

If alloc_pages() or add_to_page_cache_lru() fails, the inode is left
with an inflated i_size and possibly a partial run of pages.  As
ramfs_nommu_setattr() treats a truncate to the current i_size as a
no-op, the expansion cannot be retried and shared mmap() of the file
fails with -ENOSYS.

Setting i_size early also races with lockless readers: buffered reads
and splice do not take i_rwsem, so once the new size is visible a
concurrent read can instantiate a zero-filled folio, making the
expansion's add_to_page_cache_lru() fail with -EEXIST.

Fix this by taking mapping->invalidate_lock around the insertion,
evicting any stray folios first, and only publishing i_size once
every page is in place.

On failure, after freeing the pages that were allocated but
not inserted truncate the mapping back to empty so already inserted
pages are also disposed of.

Fixes: 642fb4d1f1dd ("[PATCH] NOMMU: Provide shared-writable mmap support on ramfs")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260523130445.1101818-1-daniel%40thingy.jp
Assisted-by: Claude:claude-5-fable # expanded my fix to address the reader race etc.
Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---

sashiko found these issues on a previous patch to fix creating memfd's.
I quickly fixed up the i_size part and then fed it to fable for review
before sending and it said the reader thing needed to be fixed too and
expanded the commit message to reflect that.

I am not sure about the locking part at all so hopefully someone can
chime in on that. Maybe sashiko will say the things it reported and
fable fixed aren't a problem after all.. :).

I tested the happy path on my mc68000 virt machine and manually stepped
through the code to make sure its actually being executed and tested on
real hardware.

 fs/ramfs/file-nommu.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index fb471bf88ab7..6819f4790065 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -80,8 +80,6 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
 	if (ret)
 		return ret;
 
-	i_size_write(inode, newsize);
-
 	/* allocate enough contiguous pages to be able to satisfy the
 	 * request */
 	pages = alloc_pages(gfp, order);
@@ -99,9 +97,15 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
 		__free_page(pages + loop);
 
 	/* clear the memory we allocated */
-	newsize = PAGE_SIZE * npages;
 	data = page_address(pages);
-	memset(data, 0, newsize);
+	memset(data, 0, PAGE_SIZE * npages);
+
+	/* block the read and splice paths from instantiating pagecache
+	 * folios whilst we build the contiguous mapping, and evict any
+	 * folio they may already have instantiated (the read path only
+	 * checks i_size after folio lookup/creation) */
+	filemap_invalidate_lock(inode->i_mapping);
+	truncate_inode_pages(inode->i_mapping, 0);
 
 	/* attach all the pages to the inode's address space */
 	for (loop = 0; loop < npages; loop++) {
@@ -120,11 +124,20 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
 		put_page(page);
 	}
 
+	/* only publish the new size once all of the backing pages are in
+	 * place, so that readers never observe a size without the pages to
+	 * back it and a failure leaves the inode unchanged at size 0 */
+	i_size_write(inode, newsize);
+	filemap_invalidate_unlock(inode->i_mapping);
 	return 0;
 
 add_error:
+	/* free the pages we hadn't inserted yet... */
 	while (loop < npages)
 		__free_page(pages + loop++);
+	/* ...and evict the ones we had; they belong to the page cache now */
+	truncate_inode_pages(inode->i_mapping, 0);
+	filemap_invalidate_unlock(inode->i_mapping);
 	return ret;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping()
  2026-07-13 11:57 [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping() Daniel Palmer
@ 2026-07-13 19:49 ` Hajime Tazaki
  2026-07-13 23:36   ` Daniel Palmer
  0 siblings, 1 reply; 4+ messages in thread
From: Hajime Tazaki @ 2026-07-13 19:49 UTC (permalink / raw)
  To: daniel; +Cc: ljs, akpm, dhowells, linux-kernel, sashiko-bot


Hello Daniel,

thanks for cc-ing me for the patch.

On Mon, 13 Jul 2026 13:57:00 +0200,
Daniel Palmer wrote:
> 
> ramfs_nommu_expand_for_mapping() sets the new i_size before it has
> allocated or inserted any of the contiguous backing pages.
> 
> If alloc_pages() or add_to_page_cache_lru() fails, the inode is left
> with an inflated i_size and possibly a partial run of pages.  As
> ramfs_nommu_setattr() treats a truncate to the current i_size as a
> no-op, the expansion cannot be retried and shared mmap() of the file
> fails with -ENOSYS.
> 
> Setting i_size early also races with lockless readers: buffered reads
> and splice do not take i_rwsem, so once the new size is visible a
> concurrent read can instantiate a zero-filled folio, making the
> expansion's add_to_page_cache_lru() fail with -EEXIST.
> 
> Fix this by taking mapping->invalidate_lock around the insertion,
> evicting any stray folios first, and only publishing i_size once
> every page is in place.
> 
> On failure, after freeing the pages that were allocated but
> not inserted truncate the mapping back to empty so already inserted
> pages are also disposed of.
> 
> Fixes: 642fb4d1f1dd ("[PATCH] NOMMU: Provide shared-writable mmap support on ramfs")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/message/20260523130445.1101818-1-daniel%40thingy.jp
> Assisted-by: Claude:claude-5-fable # expanded my fix to address the reader race etc.
> Signed-off-by: Daniel Palmer <daniel@thingy.jp>
> ---

I have tested this patch in addition to your previous patch
(20260523130445.1101818-1-daniel@thingy.jp) over LTP test with my
local nommu-revert patchset.

I run the following test (which I have never looked at thus doesn't
have much knowledge about the test itself).

https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/memfd_create/memfd_create01.c

without first patch, it indeed returns EFBIG on memfe_createfd()
syscall.  applying both patches with this test gives me still failure
with the following messages:

```
/__w/linux/linux/lib/tst_test.c:2060: TINFO: LTP version: 20260529
/__w/linux/linux/lib/tst_test.c:2063: TINFO: Tested kernel: 7.1.0-rc1-gfd4e5701bb02-dirty #1 Mon Jul 13 18:17:11 UTC 2026 um(nommu)/x86_64
/__w/linux/linux/lib/tst_kconfig.c:90: TINFO: Parsing kernel config &#x27;/proc/config.gz&#x27;
/__w/linux/linux/lib/tst_test.c:1880: TINFO: Overall timeout per run is 0h 05m 24s
/__w/linux/linux/testcases/kernel/syscalls/memfd_create/memfd_create01.c:241: TINFO: Basic tests + set/get seals
/__w/linux/linux/testcases/kernel/syscalls/memfd_create/memfd_create01.c:243: TPASS: memfd_create(ltp_memfd_create01, 2) succeeded
/__w/linux/linux/testcases/kernel/syscalls/memfd_create/memfd_create01.c:243: TPASS: ftruncate(3, 8192) succeeded
/__w/linux/linux/testcases/kernel/syscalls/memfd_create/memfd_create_common.c:212: TBROK: fcntl(3,F_GET_SEALS,...) failed: EINVAL (22)

Summary:
passed   2
failed   0
broken   1
skipped  0
warnings 0
```

which fails at the location below (in memfd_create_common.c#L207)

	int ret = SAFE_FCNTL((fd), F_GET_SEALS);

so fcntl(fd, F_GET_SEALS) always fails with nommu due to lack of
support of CONFIG_SHMEM and CONFIG_HUGETLBFS.


``` mm/memfd.c (in kernel)
static unsigned int *memfd_file_seals_ptr(struct file *file)
{
	if (shmem_file(file))
		return &SHMEM_I(file_inode(file))->seals;

#ifdef CONFIG_HUGETLBFS
	if (is_file_hugepages(file))
		return &HUGETLBFS_I(file_inode(file))->seals;
#endif

	return NULL;
}
```

so I was wondering if it is still interesting to support the syscall
(memfd_createfd) on nommu even the fcntl always fails.

My test is used on riscv-nommu (buildroot) and UML + !MMU, which gives
me the same results.

-- Hajime

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping()
  2026-07-13 19:49 ` Hajime Tazaki
@ 2026-07-13 23:36   ` Daniel Palmer
  2026-07-14  0:26     ` Hajime Tazaki
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Palmer @ 2026-07-13 23:36 UTC (permalink / raw)
  To: Hajime Tazaki; +Cc: ljs, akpm, dhowells, linux-kernel, sashiko-bot

Hi Hajime,

On Tue, 14 Jul 2026 at 04:49, Hajime Tazaki <thehajime@gmail.com> wrote:

> so I was wondering if it is still interesting to support the syscall
> (memfd_createfd) on nommu even the fcntl always fails.

I think these fcntls will also fail on MMU=y where the full tmpfs is
not enabled for size reasons. I'm not sure if it's actually possible
to detect if tmpfs is really tmpfs or not. (I have this exact issue
with a test for nolibc).

memfd_createfd is still useful because it basically allows you to
create anonymous temporary files that you can pass to another process
and mmap() in both.
I use this for doing DNS resolution in my nolibc system: A process
that needs DNS resolution creates an memfd for the result and then
forks the resolver which mmap()s the memfd, fills in the result and
then exits. This avoids having the big resolver code in everything
that needs it since I don't have any shared libraries :).

BTW: Could you share a link to your LTP fork? I will add it to my m68k
environment.

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping()
  2026-07-13 23:36   ` Daniel Palmer
@ 2026-07-14  0:26     ` Hajime Tazaki
  0 siblings, 0 replies; 4+ messages in thread
From: Hajime Tazaki @ 2026-07-14  0:26 UTC (permalink / raw)
  To: daniel; +Cc: ljs, akpm, dhowells, linux-kernel, sashiko-bot


On Tue, 14 Jul 2026 01:36:21 +0200,
Daniel Palmer wrote:
> 
> > so I was wondering if it is still interesting to support the syscall
> > (memfd_createfd) on nommu even the fcntl always fails.
> 
> I think these fcntls will also fail on MMU=y where the full tmpfs is
> not enabled for size reasons. I'm not sure if it's actually possible
> to detect if tmpfs is really tmpfs or not. (I have this exact issue
> with a test for nolibc).

thanks, I'll look into detail.

> memfd_createfd is still useful because it basically allows you to
> create anonymous temporary files that you can pass to another process
> and mmap() in both.
> I use this for doing DNS resolution in my nolibc system: A process
> that needs DNS resolution creates an memfd for the result and then
> forks the resolver which mmap()s the memfd, fills in the result and
> then exits. This avoids having the big resolver code in everything
> that needs it since I don't have any shared libraries :).

I understand, this might be a good test case with the actual use.

> BTW: Could you share a link to your LTP fork? I will add it to my m68k
> environment.

forgive me in advance :)
links are only the current working tree; it might be renamed/removed
in future.

here is a fork/branch of ltp:
https://github.com/thehajime/ltp/tree/fix-nommu

it is triggered from my UML branch with github actions (below),

https://github.com/thehajime/linux/blob/zpoline-nommu-v6.10/.github/workflows/ci.yml

using this dirty yaml file to build and execute part of testcases.

https://github.com/thehajime/linux/blob/zpoline-nommu-v6.10/.github/workflows/ci-docker-build.yml

-- Hajime

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-14  0:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 11:57 [PATCH] ramfs: nommu: fix error rollback and reader races in ramfs_nommu_expand_for_mapping() Daniel Palmer
2026-07-13 19:49 ` Hajime Tazaki
2026-07-13 23:36   ` Daniel Palmer
2026-07-14  0:26     ` Hajime Tazaki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.