[PATCH V12 00/12] famfs: the Fabric-Attached Memory File System (standalone)

[PATCH V12 00/12] famfs: the Fabric-Attached Memory File System (standalone)

[John Groves]

From: John Groves <john@groves.net>

This patch series introduces famfs as a standalone file system, as a pivot
back from fuse/libfuse, for which the porting effort took close to a year, but
then the review effort started with almost a yearlong delay, and then got
messy :D [3].

However, after years of maintaining famfs in both forms (standalone and fuse -
the famfs user space works with both) I think famfs makes more sense as a
standalone file system - and some influential maintainers have privately
expressed that opinion to me.

The most important thing to know about famfs is that it CANNOT be used as a
general purpose file system. It is for enabling file-based byte-level access
(including direct mmap) to very large (e.g. 100TB) shared/disaggregated
memory appliances - which have become available during this long process,
and which are in need of Linux support.

So famfs cannot be used by anybody who doesn't know why they need it; Making
famfs standalone means it can't affect users who don't use it. And my super-
mega-corp employer (Micron), as well as other memory companies, need it.
We do not intend to abandon it, but imagine if we did: fs/famfs/ should be
removed if the memory companies can't be bothered to maintain it.

This code base has been in active CI and used globally by early adopters and
testers of disaggregated memory. I believe it is solid.

History (skip if you lived it like I did :D)

Famfs was introduced at LPC 2023 and LSFMM 2024 [1] as a standalone file
system. The fuse odyssey started at LSFMM 24, but I think it's time to
bring that to a close. The first fuse/libfuse patches came out in April
2025 [2] - adding a substantial amount of complexity to the famfs user space,
and controversial complexity to fuse [3].

The fuse community was uncomfortable with the new metadata and fuse message
formats that famfs needs, going so far as to suggest that famfs use a BPF
program as a vma fault handler [3] to avoid the need to pass famfs file
metadata via fuse messages - an idea which, to my relief, was definitively
shot down at LSFMM 2026 [4].

In that session, I was preparing to make the pivot-to-standalone argument,
and there were others in the room who support that - but Miklos stated that
he was not opposed to merging famfs as it stands [4], leading me to back off
of the standalone pivot at the time.

However, 1) that is not how the recent patch discussions have been going [5],
and 2) I have (not solely on that basis) come to the conclusion that famfs
makes more sense as a standalone entity. In v11 [5] I dropped the
controversial famfs interleaved extent (exploding fmaps from well under a
page to megabytes), and there is still a push for adding an intermediate
virtual backing dev layer that famfs does not need and can't really live
with. I could go on...

My argument

Can we make famfs work in fuse? Yes, if we can agree some things that have
been quite challenging to agree on. Should we? I think not.

I think famfs adds complexity to fuse that will not likely see constructive
re-use. And I think fuse makes famfs worse - significantly more complex, less
adaptable to change, and less performant. And famfs files are memory - access
needs to run at memory speeds! In short the risks out-weigh the benefits.

About this patch series

I've called it V12 for recent continuity; there was a V1 and a V2 standalone
in 2024, and then V1-V11 were fuse (call this selective consistency).

Famfs depends on the 'fsdev' dax mode which landed in 7.1 - it will only run
with an fsdev-mode (aka famfs-mode) daxdev as its backing device(s).

Patch 01 is a fix to the fsdev driver that is needed by the rest of the
series.

Famfs Overview

Famfs exposes sharable disaggregated memory as a file system. Famfs consumes
shared memory from [usually shared memory] dax devices, and provides
memory-mappable files that map directly to the memory - no page cache
involvement. Famfs differs from conventional file systems in fs-dax mode,
in that it handles in-memory metadata in a sharable way (which begins with
never caching dirty shared metadata). So a famfs file system can be mounted
from multiple nodes, provided they have access to the memory.

The key performance requirement is that famfs must resolve mapping
faults with minimal overhead. This is achieved by fully caching the
file-to-devdax metadata for all active files.

Famfs remains the first fs-dax file system that is backed by devdax
rather than pmem in fs-dax mode (hence the need for the new dax mode).

The famfs user space can be found at [6]

[1] https://lwn.net/Articles/983105/ (Famfs at LSFMM 2024)
[2] https://lwn.net/Articles/1020170/ (Famfs at LSFMM 2025, with patch link)
[3] https://lwn.net/Articles/1068686/ (LWN coverage of the patch thread)
[4] https://lwn.net/Articles/1082687/ (Famfs at LSFMM 2026)
[5] https://lore.kernel.org/linux-fsdevel/0100019f7d9fbe81-6cb16662-2522-47ea-a152-fab0ee3d9b35-000000@email.amazonses.com/#b
[6] https://famfs.org

John Groves (12):
  dax: replace exported dax_dev_get() with non-allocating dax_dev_find()
  famfs: Module operations, fs_context, and mount
  famfs: Add daxdev table and dax notify_failure support
  famfs: Introduce inode_operations and super_operations
  famfs: Introduce file_operations read/write
  famfs: Introduce mmap and VM fault handling
  famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)
  famfs: iomap_begin and file-to-dax offset resolution
  famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN)
  famfs: Add runtime operation-permission (opts) framework
  famfs: Report device capacity via statfs so df works
  famfs: Add documentation

 Documentation/filesystems/famfs.rst           | 142 +++
 Documentation/filesystems/index.rst           |   1 +
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 MAINTAINERS                                   |   8 +
 drivers/dax/fsdev.c                           |  19 +
 drivers/dax/super.c                           |  38 +-
 fs/Kconfig                                    |   2 +
 fs/Makefile                                   |   1 +
 fs/famfs/Kconfig                              |  11 +
 fs/famfs/Makefile                             |   5 +
 fs/famfs/famfs_file.c                         | 983 ++++++++++++++++++
 fs/famfs/famfs_inode.c                        | 813 +++++++++++++++
 fs/famfs/famfs_internal.h                     | 162 +++
 fs/namei.c                                    |   1 +
 fs/super.c                                    |   7 +
 include/linux/dax.h                           |   7 +-
 include/linux/fs.h                            |   1 +
 include/uapi/linux/famfs_ioctl.h              | 160 +++
 include/uapi/linux/magic.h                    |   1 +
 19 files changed, 2360 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/filesystems/famfs.rst
 create mode 100644 fs/famfs/Kconfig
 create mode 100644 fs/famfs/Makefile
 create mode 100644 fs/famfs/famfs_file.c
 create mode 100644 fs/famfs/famfs_inode.c
 create mode 100644 fs/famfs/famfs_internal.h
 create mode 100644 include/uapi/linux/famfs_ioctl.h

-- 
2.53.0

[PATCH V12 01/12] dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

[John Groves]

From: John Groves <John@Groves.net>

This fix is in response to a Sashiko review, and some subsequent
analysis.

dax_dev_get() uses iget5_locked() which creates a new inode if no
matching one exists. This is correct for the internal caller
(alloc_dax), but dangerous for external callers that look up devices
from user-supplied or metadata-supplied dev_t values:

1. A new inode is created with DAXDEV_ALIVE set but no backing driver,
   no ops, and no IDA-allocated minor number.

2. On teardown, dax_destroy_inode() warns because kill_dax() was never
   called, and dax_free_inode() calls ida_free() for a minor that was
   never ida_alloc'd -- potentially freeing the minor of a real device.

Add dax_dev_find() which uses ilookup5() for lookup-only semantics:
it returns an existing dax_device with an elevated inode reference, or
NULL if no device with the given dev_t exists. It never creates inodes.
A dax_alive() check under dax_read_lock() guards against returning a
device that is concurrently being torn down by kill_dax().

Make dax_dev_get() static again (internal to super.c for alloc_dax),
export dax_dev_find() instead, and update the two external callers
(famfs_inode.c, famfs.c). Also add the missing CONFIG_DAX=n stub.

About the 'fixes' tag: this removes the export of dax_dev_get(),
which was flawed, and replaces is with dax_dev_find(). It feels like
the fixes tag makes sense for correcting an ABI error.

Fixes: 2ae624d5a555d ("dax: export dax_dev_get()")

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: John Groves <john@groves.net>
---
 drivers/dax/super.c | 38 ++++++++++++++++++++++++++++++++++++--
 include/linux/dax.h |  6 +++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 25cf99dd9360..0fc14d4f8198 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -521,7 +521,7 @@ static int dax_set(struct inode *inode, void *data)
 	return 0;
 }
 
-struct dax_device *dax_dev_get(dev_t devt)
+static struct dax_device *dax_dev_get(dev_t devt)
 {
 	struct dax_device *dax_dev;
 	struct inode *inode;
@@ -544,7 +544,41 @@ struct dax_device *dax_dev_get(dev_t devt)
 
 	return dax_dev;
 }
-EXPORT_SYMBOL_GPL(dax_dev_get);
+
+/**
+ * dax_dev_find - look up an existing dax_device by dev_t
+ * @devt: the device number to find
+ *
+ * Returns a dax_device with an elevated inode reference, or NULL if no
+ * device with the given dev_t exists. Unlike dax_dev_get(), this never
+ * allocates a new inode -- it is safe for external callers that are looking
+ * up devices from user-supplied or metadata-supplied dev_t values.
+ *
+ * Caller must put_dax() the returned device when done.
+ */
+struct dax_device *dax_dev_find(dev_t devt)
+{
+	struct dax_device *dax_dev;
+	struct inode *inode;
+	int id;
+
+	inode = ilookup5(dax_superblock, hash_32(devt + DAXFS_MAGIC, 31),
+			 dax_test, &devt);
+	if (!inode)
+		return NULL;
+
+	dax_dev = to_dax_dev(inode);
+	id = dax_read_lock();
+	if (!dax_alive(dax_dev)) {
+		dax_read_unlock(id);
+		iput(inode);
+		return NULL;
+	}
+	dax_read_unlock(id);
+
+	return dax_dev;
+}
+EXPORT_SYMBOL_GPL(dax_dev_find);
 
 struct dax_device *alloc_dax(void *private, const struct dax_operations *ops)
 {
diff --git a/include/linux/dax.h b/include/linux/dax.h
index fe6c3ded1b50..29113eb95e72 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -54,7 +54,7 @@ struct dax_device *alloc_dax(void *private, const struct dax_operations *ops);
 void *dax_holder(struct dax_device *dax_dev);
 void put_dax(struct dax_device *dax_dev);
 void kill_dax(struct dax_device *dax_dev);
-struct dax_device *dax_dev_get(dev_t devt);
+struct dax_device *dax_dev_find(dev_t devt);
 void dax_write_cache(struct dax_device *dax_dev, bool wc);
 bool dax_write_cache_enabled(struct dax_device *dax_dev);
 bool dax_synchronous(struct dax_device *dax_dev);
@@ -92,6 +92,10 @@ static inline void put_dax(struct dax_device *dax_dev)
 static inline void kill_dax(struct dax_device *dax_dev)
 {
 }
+static inline struct dax_device *dax_dev_find(dev_t devt)
+{
+	return NULL;
+}
 static inline void dax_write_cache(struct dax_device *dax_dev, bool wc)
 {
 }
-- 
2.53.0

Re: [PATCH V12 01/12] dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race Condition / Uninitialized Data Access during DAX device allocation.
--

--- Patch [1]: [PATCH V12 01/12] dax: replace exported dax_dev_get() with non-allocating dax_dev_find() ---
commit ff957b44b15e4c0e4aaedcf9cd2d12118ab1bb27
Author: John Groves <John@Groves.net>

dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

This patch introduces dax_dev_find() to look up existing DAX devices
without allocating a new inode, preventing empty inodes from being created
when external callers provide user-supplied device numbers. It also makes
dax_dev_get() static and updates external references.

> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index 25cf99dd9360b..0fc14d4f81989 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
[ ... ]
> @@ -544,7 +544,41 @@ struct dax_device *dax_dev_get(dev_t devt)
>  
>  	return dax_dev;
>  }
> -EXPORT_SYMBOL_GPL(dax_dev_get);
> +
> +/**
> + * dax_dev_find - look up an existing dax_device by dev_t
[ ... ]
> + */
> +struct dax_device *dax_dev_find(dev_t devt)
> +{
> +	struct dax_device *dax_dev;
> +	struct inode *inode;
> +	int id;
> +
> +	inode = ilookup5(dax_superblock, hash_32(devt + DAXFS_MAGIC, 31),
> +			 dax_test, &devt);
> +	if (!inode)
> +		return NULL;
> +
> +	dax_dev = to_dax_dev(inode);
> +	id = dax_read_lock();
> +	if (!dax_alive(dax_dev)) {
> +		dax_read_unlock(id);
> +		iput(inode);
> +		return NULL;
> +	}
> +	dax_read_unlock(id);
> +
> +	return dax_dev;
> +}

[Severity: High]
This isn't a bug introduced by this patch, but could a concurrent caller of
dax_dev_find() obtain a partially initialized device here?

In dax_dev_get(), unlock_new_inode() clears I_NEW and makes the DAX device
visible to ilookup5(). However, alloc_dax() sets the ops and private
pointers after dax_dev_get() returns:

alloc_dax() {
    ...
    dax_dev = dax_dev_get(devt);
    if (!dax_dev)
        goto err_dev;

    dax_dev->ops = ops;
    dax_dev->private = private;
    ...
}

Because dax_dev_find() retrieves the device from the inode cache and only
checks if DAXDEV_ALIVE is set, could an external caller using a guessed
device number observe NULL ops or private pointers? 

If the external caller attempts to use the returned device before alloc_dax()
completes, could this lead to a NULL pointer dereference and kernel panic?

> +EXPORT_SYMBOL_GPL(dax_dev_find);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=1

[PATCH V12 02/12] famfs: Module operations, fs_context, and mount

[John Groves]

From: John Groves <john@groves.net>

Start building up from the famfs module operations. This commit
includes the following:

* Register as a file system
* Parse mount parameters
* Allocate or find (and initialize) a superblock via famfs_get_tree()
* Lookup the host dax device, and bail if it's in use (or not dax)
* Add Kconfig and Makefile misc to build famfs
* Add FAMFS_SUPER_MAGIC to include/uapi/linux/magic.h
* Add export of fs/namei.c:may_open_dev(), which famfs needs to call
* Update MAINTAINERS file for the fs/famfs/ path

The following exports had to happen to enable famfs:

* This adds the new fs/super.c:kill_char_super() - the other kill*super
  helpers were not quite right.

This commit builds but is otherwise too incomplete to run

Signed-off-by: John Groves <john@groves.net>
---
 MAINTAINERS                |   7 +
 fs/Kconfig                 |   2 +
 fs/Makefile                |   1 +
 fs/famfs/Kconfig           |  11 ++
 fs/famfs/Makefile          |   5 +
 fs/famfs/famfs_inode.c     | 293 +++++++++++++++++++++++++++++++++++++
 fs/famfs/famfs_internal.h  |  32 ++++
 fs/namei.c                 |   1 +
 fs/super.c                 |   7 +
 include/linux/fs.h         |   1 +
 include/uapi/linux/magic.h |   1 +
 11 files changed, 361 insertions(+)
 create mode 100644 fs/famfs/Kconfig
 create mode 100644 fs/famfs/Makefile
 create mode 100644 fs/famfs/famfs_inode.c
 create mode 100644 fs/famfs/famfs_internal.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a674e36529f7..ca7b90a8f0a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9905,6 +9905,13 @@ F:	Documentation/networking/failover.rst
 F:	include/net/failover.h
 F:	net/core/failover.c
 
+FAMFS [Fabric-Attached Memory File System]
+M:	John Groves <John@Groves.net>
+L:	linux-fsdevel@vger.kernel.org
+L:	linux-cxl@vger.kernel.org
+S:	Supported
+F:	fs/famfs/
+
 FANOTIFY
 M:	Jan Kara <jack@suse.cz>
 R:	Amir Goldstein <amir73il@gmail.com>
diff --git a/fs/Kconfig b/fs/Kconfig
index cf6ae64776e6..2db647accc00 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -131,6 +131,8 @@ source "fs/autofs/Kconfig"
 source "fs/fuse/Kconfig"
 source "fs/overlayfs/Kconfig"
 
+source "fs/famfs/Kconfig"
+
 menu "Caches"
 
 source "fs/netfs/Kconfig"
diff --git a/fs/Makefile b/fs/Makefile
index 89a8a9d207d1..f49f9a000210 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -129,3 +129,4 @@ obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
 obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
 obj-$(CONFIG_BPF_LSM)		+= bpf_fs_kfuncs.o
 obj-$(CONFIG_RESCTRL_FS)	+= resctrl/
+obj-$(CONFIG_FAMFS)		+= famfs/
diff --git a/fs/famfs/Kconfig b/fs/famfs/Kconfig
new file mode 100644
index 000000000000..ed40cf8b0592
--- /dev/null
+++ b/fs/famfs/Kconfig
@@ -0,0 +1,11 @@
+
+
+config FAMFS
+       tristate "famfs: shared memory file system"
+       depends on DEV_DAX && FS_DAX && DEV_DAX_FSDEV
+       default m if DEV_DAX && FS_DAX && DEV_DAX_FSDEV
+       help
+	  Support for the famfs file system. Famfs is a dax file system that
+	  can support scale-out shared access to fabric-attached memory
+	  (e.g. CXL shared memory). Famfs is not a general purpose file system;
+	  it is an enabler for data sets in shared memory.
diff --git a/fs/famfs/Makefile b/fs/famfs/Makefile
new file mode 100644
index 000000000000..62230bcd6793
--- /dev/null
+++ b/fs/famfs/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_FAMFS) += famfs.o
+
+famfs-y := famfs_inode.o
diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
new file mode 100644
index 000000000000..c299a90912a5
--- /dev/null
+++ b/fs/famfs/famfs_inode.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * famfs - dax file system for shared fabric-attached memory
+ *
+ * Copyright 2023-2024 Micron Technology, inc
+ *
+ * This file system, originally based on ramfs the dax support from xfs,
+ * is intended to allow multiple host systems to mount a common file system
+ * view of dax files that map to shared memory.
+ */
+
+#include <linux/fs.h>
+#include <linux/cleanup.h>
+#include <linux/time.h>
+#include <linux/init.h>
+#include <linux/string.h>
+#include <linux/parser.h>
+#include <linux/magic.h>
+#include <linux/slab.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
+#include <linux/dax.h>
+#include <linux/hugetlb.h>
+#include <linux/iomap.h>
+#include <linux/path.h>
+#include <linux/namei.h>
+
+#include "famfs_internal.h"
+
+#define FAMFS_DEFAULT_MODE	0755
+
+static struct inode *famfs_get_inode(
+			struct super_block *sb,
+			const struct inode *dir,
+			umode_t mode, dev_t dev)
+{
+	struct inode *inode = new_inode(sb);
+	struct timespec64 tv;
+
+	if (!inode)
+		return NULL;
+
+	inode->i_ino = get_next_ino();
+	inode_init_owner(&nop_mnt_idmap, inode, dir, mode);
+	inode->i_mapping->a_ops = &ram_aops;
+	mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+	mapping_set_unevictable(inode->i_mapping);
+	tv = inode_set_ctime_current(inode);
+	inode_set_mtime_to_ts(inode, tv);
+	inode_set_atime_to_ts(inode, tv);
+
+	switch (mode & S_IFMT) {
+	default:
+		init_special_inode(inode, mode, dev);
+		break;
+	case S_IFREG:
+		inode->i_op = NULL /* famfs_file_inode_operations */;
+		inode->i_fop = NULL /* &famfs_file_operations */;
+		break;
+	case S_IFDIR:
+		inode->i_op = NULL /* famfs_dir_inode_operations */;
+		inode->i_fop = &simple_dir_operations;
+
+		/* Directory inodes start off with i_nlink == 2 (for ".") */
+		inc_nlink(inode);
+		break;
+	case S_IFLNK:
+		inode->i_op = &page_symlink_inode_operations;
+		inode_nohighmem(inode);
+		break;
+	}
+	return inode;
+}
+
+/*
+ * famfs dax_operations (for famfs-mode dax)
+ */
+/*****************************************************************************
+ * fs_context_operations
+ */
+
+static void
+famfs_fill_super(struct super_block *sb, struct fs_context *fc)
+{
+	sb->s_maxbytes		= MAX_LFS_FILESIZE;
+	sb->s_blocksize		= PAGE_SIZE;
+	sb->s_blocksize_bits	= PAGE_SHIFT;
+	sb->s_magic		= FAMFS_SUPER_MAGIC;
+	sb->s_op		= NULL /* famfs_super_ops */;
+	sb->s_time_gran		= 1;
+}
+
+int
+lookup_daxdev(const char *pathname, dev_t *devno)
+{
+	struct inode *inode;
+	struct path path;
+	int err;
+
+	if (!pathname || !*pathname)
+		return -EINVAL;
+
+	err = kern_path(pathname, LOOKUP_FOLLOW, &path);
+	if (err)
+		return err;
+
+	inode = d_backing_inode(path.dentry);
+	if (!S_ISCHR(inode->i_mode)) {
+		err = -EINVAL;
+		goto out_path_put;
+	}
+
+	if (!may_open_dev(&path)) {
+		err = -EACCES;
+		goto out_path_put;
+	}
+
+	/* i_rdev is the char dev_t; fs_dax_get() confirms it is dax later */
+	*devno = inode->i_rdev;
+
+out_path_put:
+	path_put(&path);
+	return err;
+}
+
+static int
+famfs_get_tree(struct fs_context *fc)
+{
+	struct famfs_fs_info *fsi = fc->s_fs_info;
+	struct super_block *sb;
+	struct inode *inode;
+	dev_t daxdevno;
+	int err;
+
+	err = lookup_daxdev(fc->source, &daxdevno);
+	if (err)
+		return err;
+
+	/* This will set sb->s_dev=daxdevno */
+	sb = sget_dev(fc, daxdevno);
+	if (IS_ERR(sb)) {
+		pr_debug("%s: sget_dev error\n", __func__);
+		return PTR_ERR(sb);
+	}
+
+	if (sb->s_root) {
+		pr_debug("%s: found a matching superblock for %s\n",
+			__func__, fc->source);
+
+		/* We don't expect to find a match by dev_t; if we do, it must
+		 * already be mounted, so we bail
+		 */
+		err = -EBUSY;
+		goto deactivate_out;
+	} else {
+		pr_debug("%s: initializing new superblock for %s\n",
+			__func__, fc->source);
+		famfs_fill_super(sb, fc);
+	}
+
+	inode = famfs_get_inode(sb, NULL, S_IFDIR | fsi->mount_opts.mode, 0);
+	sb->s_root = d_make_root(inode);
+	if (!sb->s_root) {
+		pr_debug("%s: d_make_root() failed\n", __func__);
+		err = -ENOMEM;
+		goto deactivate_out;
+	}
+
+	sb->s_flags |= SB_ACTIVE;
+
+	WARN_ON(fc->root);
+	fc->root = dget(sb->s_root);
+	return 0;
+
+deactivate_out:
+	pr_debug("%s: deactivating sb=%llx\n", __func__, (u64)sb);
+	deactivate_locked_super(sb);
+	return err;
+}
+
+/*****************************************************************************/
+
+enum famfs_param {
+	Opt_mode,
+	Opt_dax,
+};
+
+const struct fs_parameter_spec famfs_fs_parameters[] = {
+	fsparam_u32oct("mode",	  Opt_mode),
+	fsparam_string("dax",     Opt_dax),
+	{}
+};
+
+static int famfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct famfs_fs_info *fsi = fc->s_fs_info;
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, famfs_fs_parameters, param, &result);
+	if (opt == -ENOPARAM) {
+		opt = vfs_parse_fs_param_source(fc, param);
+		if (opt != -ENOPARAM)
+			return opt;
+
+		return 0;
+	}
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_mode:
+		fsi->mount_opts.mode = result.uint_32 & S_IALLUGO;
+		break;
+	case Opt_dax:
+		if (strcmp(param->string, "always"))
+			pr_debug("%s: invalid dax mode %s\n",
+				  __func__, param->string);
+		break;
+	}
+
+	return 0;
+}
+
+static void famfs_free_fc(struct fs_context *fc)
+{
+	kfree(fc->s_fs_info);
+}
+
+static const struct fs_context_operations famfs_context_ops = {
+	.free		= famfs_free_fc,
+	.parse_param	= famfs_parse_param,
+	.get_tree	= famfs_get_tree,
+};
+
+static int famfs_init_fs_context(struct fs_context *fc)
+{
+	struct famfs_fs_info *fsi;
+
+	fsi = kzalloc_obj(*fsi, GFP_KERNEL);
+	if (!fsi)
+		return -ENOMEM;
+
+	fsi->mount_opts.mode = FAMFS_DEFAULT_MODE;
+	fc->s_fs_info        = fsi;
+	fc->ops              = &famfs_context_ops;
+	return 0;
+}
+
+static void famfs_kill_sb(struct super_block *sb)
+{
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+
+	kill_char_super(sb);
+
+	kfree(fsi);
+	sb->s_fs_info = NULL;
+}
+
+#define MODULE_NAME "famfs"
+static struct file_system_type famfs_fs_type = {
+	.name		  = MODULE_NAME,
+	.init_fs_context  = famfs_init_fs_context,
+	.parameters	  = famfs_fs_parameters,
+	.kill_sb	  = famfs_kill_sb,
+	.fs_flags	  = FS_REQUIRES_DEV,
+};
+
+/******************************************************************************
+ * Module stuff
+ */
+static int __init init_famfs_fs(void)
+{
+	int rc;
+
+	rc = register_filesystem(&famfs_fs_type);
+
+	return rc;
+}
+
+static void
+__exit famfs_exit(void)
+{
+	unregister_filesystem(&famfs_fs_type);
+	pr_info("%s: unregistered\n", __func__);
+}
+
+fs_initcall(init_famfs_fs);
+module_exit(famfs_exit);
+
+MODULE_AUTHOR("John Groves");
+MODULE_DESCRIPTION("Fabric-Attached Memory File System: see famfs.org");
+MODULE_LICENSE("GPL");
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
new file mode 100644
index 000000000000..6f481d79ed88
--- /dev/null
+++ b/fs/famfs/famfs_internal.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * famfs - dax file system for shared fabric-attached memory
+ *
+ * Copyright 2023-2024 Micron Technology, Inc.
+ *
+ * This file system, originally based on ramfs the dax support from xfs,
+ * is intended to allow multiple host systems to mount a common file system
+ * view of dax files that map to shared memory.
+ */
+#ifndef FAMFS_INTERNAL_H
+#define FAMFS_INTERNAL_H
+
+struct famfs_mount_opts {
+	umode_t mode;
+};
+
+/**
+ * @famfs_fs_info
+ *
+ * @mount_opts:  The mount options
+ * @deverror:    True if the dax device has called our notify_failure entry
+ *               point, or if other "shutdown" conditions exist
+ */
+struct famfs_fs_info {
+	struct famfs_mount_opts   mount_opts;
+	bool                      deverror;
+};
+
+int lookup_daxdev(const char *pathname, dev_t *devno);
+
+#endif /* FAMFS_INTERNAL_H */
diff --git a/fs/namei.c b/fs/namei.c
index 19ce43c9a6e6..d67194e89963 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4232,6 +4232,7 @@ bool may_open_dev(const struct path *path)
 	return !(path->mnt->mnt_flags & MNT_NODEV) &&
 		!(path->mnt->mnt_sb->s_iflags & SB_I_NODEV);
 }
+EXPORT_SYMBOL(may_open_dev);
 
 static int may_open(struct mnt_idmap *idmap, const struct path *path,
 		    int acc_mode, int flag)
diff --git a/fs/super.c b/fs/super.c
index a8fd61136aaf..822ed205d07b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1235,6 +1235,13 @@ void kill_anon_super(struct super_block *sb)
 }
 EXPORT_SYMBOL(kill_anon_super);
 
+void kill_char_super(struct super_block *sb)
+{
+	generic_shutdown_super(sb);
+	kill_super_notify(sb);
+}
+EXPORT_SYMBOL(kill_char_super);
+
 int set_anon_super_fc(struct super_block *sb, struct fs_context *fc)
 {
 	return set_anon_super(sb, NULL);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 50ce731a2b78..a8b030d6f218 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2328,6 +2328,7 @@ void retire_super(struct super_block *sb);
 void generic_shutdown_super(struct super_block *sb);
 void kill_block_super(struct super_block *sb);
 void kill_anon_super(struct super_block *sb);
+void kill_char_super(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
 void deactivate_locked_super(struct super_block *sb);
 int set_anon_super(struct super_block *s, void *data);
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index 4f2da935a76c..e644e1fd49bd 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -38,6 +38,7 @@
 #define OVERLAYFS_SUPER_MAGIC	0x794c7630
 #define FUSE_SUPER_MAGIC	0x65735546
 #define BCACHEFS_SUPER_MAGIC	0xca451a4e
+#define FAMFS_SUPER_MAGIC	0x87b282ff
 
 #define MINIX_SUPER_MAGIC	0x137F		/* minix v1 fs, 14 char names */
 #define MINIX_SUPER_MAGIC2	0x138F		/* minix v1 fs, 30 char names */
-- 
2.53.0

Re: [PATCH V12 02/12] famfs: Module operations, fs_context, and mount

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing `.owner = THIS_MODULE` in `famfs_fs_type`.
- [High] Missing DAX device validation and reference counting risks Use-After-Free and TOCTOU races.
- [Medium] Silent ignoring of unrecognized and invalid mount parameters in `famfs_parse_param()`.
- [Medium] Bypassing MAC and DAC permission checks on the character device node during mount.
- [Low] Global symbol `lookup_daxdev` lacks a subsystem prefix.
--

--- Patch [2]: [PATCH V12 02/12] famfs: Module operations, fs_context, and mount ---
commit fdda77a79254c638517e00787a7301ff9eaab82f
Author: John Groves <john@groves.net>

famfs: Module operations, fs_context, and mount

Start building up from the famfs module operations. Register as a file
system, parse mount parameters, allocate superblock via famfs_get_tree(),
and lookup the host dax device.

> diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
> new file mode 100644
> index 0000000000000..c299a90912a5c
> --- /dev/null
> +++ b/fs/famfs/famfs_inode.c

[ ... ]

> +static void
> +famfs_fill_super(struct super_block *sb, struct fs_context *fc)
> +{
> +	sb->s_maxbytes		= MAX_LFS_FILESIZE;
> +	sb->s_blocksize		= PAGE_SIZE;
> +	sb->s_blocksize_bits	= PAGE_SHIFT;
> +	sb->s_magic		= FAMFS_SUPER_MAGIC;
> +	sb->s_op		= NULL /* famfs_super_ops */;
> +	sb->s_time_gran		= 1;
> +}
> +
> +int
> +lookup_daxdev(const char *pathname, dev_t *devno)

[Severity: Low]
Should this globally visible function have a subsystem-specific prefix like
famfs_lookup_daxdev to avoid polluting the global namespace?

> +{
> +	struct inode *inode;
> +	struct path path;
> +	int err;
> +
> +	if (!pathname || !*pathname)
> +		return -EINVAL;
> +
> +	err = kern_path(pathname, LOOKUP_FOLLOW, &path);
> +	if (err)
> +		return err;
> +
> +	inode = d_backing_inode(path.dentry);
> +	if (!S_ISCHR(inode->i_mode)) {
> +		err = -EINVAL;
> +		goto out_path_put;
> +	}
> +
> +	if (!may_open_dev(&path)) {
> +		err = -EACCES;
> +		goto out_path_put;
> +	}

[Severity: Medium]
Does this bypass MAC and DAC permission checks on the character device node?

By only resolving the path with kern_path() and checking may_open_dev(),
the code skips the actual file open operation. Would this allow a user with
mount capabilities to bypass security policies that restrict access to the
underlying device?

> +
> +	/* i_rdev is the char dev_t; fs_dax_get() confirms it is dax later */
> +	*devno = inode->i_rdev;

[Severity: High]
Is the underlying DAX device structure pinned to prevent it from being freed?

The comment mentions fs_dax_get() will confirm it is DAX later, but it
doesn't appear to be called anywhere in the codebase to acquire the
device. Could this allow the backing driver to be unloaded or hot-unplugged
while the filesystem is mounted, resulting in a use-after-free on
subsequent access?

> +
> +out_path_put:
> +	path_put(&path);
> +	return err;
> +}

[ ... ]

> +static int famfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +	struct famfs_fs_info *fsi = fc->s_fs_info;
> +	struct fs_parse_result result;
> +	int opt;
> +
> +	opt = fs_parse(fc, famfs_fs_parameters, param, &result);
> +	if (opt == -ENOPARAM) {
> +		opt = vfs_parse_fs_param_source(fc, param);
> +		if (opt != -ENOPARAM)
> +			return opt;
> +
> +		return 0;
> +	}

[Severity: Medium]
Does returning 0 here silently ignore unrecognized mount parameters?

The VFS layer relies on fs_parse() returning -ENOPARAM to indicate an
unhandled parameter so it can alert the user. Does catching the error and
returning 0 hide misconfigurations from userspace?

> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case Opt_mode:
> +		fsi->mount_opts.mode = result.uint_32 & S_IALLUGO;
> +		break;
> +	case Opt_dax:
> +		if (strcmp(param->string, "always"))
> +			pr_debug("%s: invalid dax mode %s\n",
> +				  __func__, param->string);
> +		break;

[Severity: Medium]
Should this return an error code instead of breaking and eventually
returning 0 when an invalid DAX mode is provided?

> +	}
> +
> +	return 0;
> +}

[ ... ]

> +static void famfs_kill_sb(struct super_block *sb)
> +{
> +	struct famfs_fs_info *fsi = sb->s_fs_info;
> +
> +	kill_char_super(sb);
> +
> +	kfree(fsi);
> +	sb->s_fs_info = NULL;
> +}
> +
> +#define MODULE_NAME "famfs"
> +static struct file_system_type famfs_fs_type = {
> +	.name		  = MODULE_NAME,
> +	.init_fs_context  = famfs_init_fs_context,
> +	.parameters	  = famfs_fs_parameters,
> +	.kill_sb	  = famfs_kill_sb,
> +	.fs_flags	  = FS_REQUIRES_DEV,
> +};

[Severity: High]
Is the .owner field missing from this file_system_type structure?

Without .owner = THIS_MODULE, the VFS will not increment the module
reference count when a famfs filesystem is mounted. Could this allow the
module to be unexpectedly unloaded via rmmod while active superblocks and
inodes are still in use, leading to a kernel oops and use-after-free
vulnerabilities?

> +
> +/******************************************************************************
> + * Module stuff
> + */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=2

[PATCH V12 03/12] famfs: Add daxdev table and dax notify_failure support

[John Groves]

From: John Groves <John@Groves.net>

Famfs file systems can span multiple dax devices, and daxdevs are stored
in the daxdev_table. This adds the basic table structure, primtives and
serialization code. Famfs file extents reference daxdevs by index, which
is a cluster invariant maintained by user space.

We also add dax_holder_operations and a notify_failure handler, which
is necessary to properly "open" a famfs-mode daxdev.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_inode.c    | 234 ++++++++++++++++++++++++++++++++++++++
 fs/famfs/famfs_internal.h |  46 ++++++++
 2 files changed, 280 insertions(+)

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index c299a90912a5..ad71e5e7a8e3 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -75,6 +75,225 @@ static struct inode *famfs_get_inode(
 /*
  * famfs dax_operations (for famfs-mode dax)
  */
+static void famfs_set_daxdev_err(struct famfs_fs_info *fsi,
+				 struct dax_device *dax_devp);
+
+static int
+famfs_dax_notify_failure(
+		struct dax_device *dax_dev, u64 offset,
+		u64 len, int mf_flags)
+{
+	struct super_block *sb = dax_holder(dax_dev);
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+
+	pr_err("%s: offset=%lld len=%llu flags=%x\n", __func__,
+	       offset, len, mf_flags);
+
+	/*
+	 * Record the error on the specific daxdev and, near-term, shut the
+	 * mount down: famfs_set_daxdev_err() also sets fsi->deverror so
+	 * subsequent famfs operations fail. The resolver's per-daxdev
+	 * famfs_dax_err() check remains and can make this finer later.
+	 */
+	famfs_set_daxdev_err(fsi, dax_dev);
+
+	return 0;
+}
+
+static const struct dax_holder_operations famfs_dax_holder_ops = {
+	.notify_failure		= famfs_dax_notify_failure,
+};
+
+/*
+ * Allocate the daxdev table on first use (idempotent via cmpxchg).
+ */
+int famfs_devlist_alloc(struct famfs_fs_info *fsi)
+{
+	struct famfs_dax_devlist *devlist;
+
+	if (fsi->dax_devlist)
+		return 0;
+
+	devlist = kcalloc(1, sizeof(*devlist), GFP_KERNEL);
+	if (!devlist)
+		return -ENOMEM;
+
+	devlist->nslots = FAMFS_MAX_DAXDEVS;
+	devlist->devlist = kcalloc(FAMFS_MAX_DAXDEVS, sizeof(struct famfs_daxdev),
+				   GFP_KERNEL);
+	if (!devlist->devlist) {
+		kfree(devlist);
+		return -ENOMEM;
+	}
+
+	/* If another thread allocated it first, drop ours */
+	if (cmpxchg(&fsi->dax_devlist, NULL, devlist) != NULL) {
+		kfree(devlist->devlist);
+		kfree(devlist);
+	}
+
+	return 0;
+}
+
+/*
+ * famfs_install_daxdev() - exclusively acquire a resolved daxdev and publish
+ * it in the table at @index. Slot 0 is the mount primary; slots 1..n come from
+ * the daxdev-open ioctl.
+ *
+ * Serializes with concurrent installers under devlist_sem and rechecks
+ * ->valid, so re-registering an already-installed slot is idempotent. A daxdev
+ * is entered in the table only once it has been exclusively acquired via
+ * fs_dax_get() (with the super_block as the holder); on failure the
+ * dax_dev_find() reference is released and the slot is left invalid. @name may
+ * be NULL (the ioctl path passes no pathname).
+ */
+int famfs_install_daxdev(
+		struct famfs_fs_info *fsi,
+		struct super_block *sb,
+		u64 index,
+		dev_t devno,
+		const char *name)
+{
+	struct famfs_daxdev *daxdev;
+	int rc = 0;
+
+	if (index >= fsi->dax_devlist->nslots) {
+		pr_debug("%s: index(%llu) >= nslots(%d)\n",
+		       __func__, index, fsi->dax_devlist->nslots);
+		return -EINVAL;
+	}
+
+	scoped_guard(rwsem_write, &fsi->devlist_sem) {
+		daxdev = &fsi->dax_devlist->devlist[index];
+
+		/* Installed already by a concurrent (or repeated) open */
+		if (daxdev->valid)
+			return 0;
+
+		/*
+		 * A prior attempt already determined this daxdev cannot be
+		 * exclusively acquired (see the fs_dax_get() failure handling
+		 * below). Don't thrash on fs_dax_get(); fail fast.
+		 */
+		if (daxdev->dax_err)
+			return -EIO;
+
+		daxdev->devp = dax_dev_find(devno);
+		if (!daxdev->devp) {
+			pr_debug("%s: device %u:%u not found or not dax\n",
+				__func__, MAJOR(devno), MINOR(devno));
+			return -ENODEV;
+		}
+
+		rc = fs_dax_get(daxdev->devp, sb, &famfs_dax_holder_ops);
+		if (rc) {
+			/*
+			 * Distinguish a lost race from a real failure. -EBUSY
+			 * with the daxdev already held by *this* super_block
+			 * means a concurrent acquire won and will publish the
+			 * slot valid: not an error, and must not be cached as
+			 * dax_err. Any other failure is permanent for this
+			 * mount, so record dax_err to stop re-acquiring it.
+			 */
+			if (!(rc == -EBUSY && dax_holder(daxdev->devp) == sb)) {
+				pr_debug("%s: fs_dax_get(%u:%u) failed rc=%d\n",
+				       __func__, MAJOR(devno), MINOR(devno), rc);
+				daxdev->dax_err = true;
+			}
+			put_dax(daxdev->devp);
+			daxdev->devp = NULL;
+			return rc;
+		}
+
+		daxdev->devno = devno;
+		if (name) {
+			daxdev->name = kstrdup(name, GFP_KERNEL);
+			if (!daxdev->name) {
+				fs_put_dax(daxdev->devp, sb);
+				put_dax(daxdev->devp);
+				daxdev->devp = NULL;
+				return -ENOMEM;
+			}
+		}
+
+		wmb(); /* All other fields must be visible before valid */
+		daxdev->valid = 1;
+	}
+
+	return 0;
+}
+
+/*
+ * Release every daxdev in the table and free it. Detach the table under
+ * devlist_sem so a notify_failure racing teardown either runs first against
+ * the live table or observes dax_devlist == NULL and bails.
+ */
+static void famfs_devlist_free(
+			struct famfs_fs_info *fsi,
+			struct super_block *sb)
+{
+	struct famfs_dax_devlist *devlist __free(kfree) = NULL;
+	int i;
+
+	scoped_guard(rwsem_write, &fsi->devlist_sem) {
+		devlist = fsi->dax_devlist;
+		fsi->dax_devlist = NULL;
+	}
+
+	if (!devlist || !devlist->devlist)
+		return;
+
+	for (i = 0; i < devlist->nslots; i++) {
+		struct famfs_daxdev *dd = &devlist->devlist[i];
+
+		if (!dd->valid)
+			continue;
+
+		if (dd->devp) {
+			if (!dd->dax_err)
+				fs_put_dax(dd->devp, sb);
+			put_dax(dd->devp);
+		}
+		kfree(dd->name);
+	}
+	kfree(devlist->devlist);
+}
+
+/*
+ * Record a memory error on the daxdev matching @dax_devp. Searches the table
+ * under the write lock (which serializes against famfs_devlist_free()).
+ */
+static void famfs_set_daxdev_err(
+			struct famfs_fs_info *fsi,
+			struct dax_device *dax_devp)
+{
+	int i;
+
+	scoped_guard(rwsem_write, &fsi->devlist_sem) {
+		if (!fsi->dax_devlist)
+			return;
+		for (i = 0; i < fsi->dax_devlist->nslots; i++) {
+			struct famfs_daxdev *dd = &fsi->dax_devlist->devlist[i];
+
+			if (!dd->valid || dd->devp != dax_devp)
+				continue;
+
+			dd->error = true;
+			/*
+			 * Near-term policy: any daxdev memory error shuts down
+			 * the whole mount. Finer per-daxdev handling (via
+			 * famfs_dax_err() in the resolver) already exists and
+			 * can supersede this later.
+			 */
+			fsi->deverror = true;
+			pr_err("%s: memory error on daxdev %s (%d)\n",
+			       __func__, dd->name, i);
+			return;
+		}
+	}
+	pr_debug("%s: memory error on unrecognized daxdev\n", __func__);
+}
+
 /*****************************************************************************
  * fs_context_operations
  */
@@ -158,6 +377,18 @@ famfs_get_tree(struct fs_context *fc)
 		famfs_fill_super(sb, fc);
 	}
 
+	/* Install the primary daxdev (from the mount device) at slot 0 */
+	err = famfs_devlist_alloc(fsi);
+	if (err)
+		goto deactivate_out;
+
+	err = famfs_install_daxdev(fsi, sb, 0, daxdevno, fc->source);
+	if (err) {
+		pr_err("%s: failed to install primary daxdev %s\n",
+		       __func__, fc->source);
+		goto deactivate_out;
+	}
+
 	inode = famfs_get_inode(sb, NULL, S_IFDIR | fsi->mount_opts.mode, 0);
 	sb->s_root = d_make_root(inode);
 	if (!sb->s_root) {
@@ -241,6 +472,7 @@ static int famfs_init_fs_context(struct fs_context *fc)
 	if (!fsi)
 		return -ENOMEM;
 
+	init_rwsem(&fsi->devlist_sem);
 	fsi->mount_opts.mode = FAMFS_DEFAULT_MODE;
 	fc->s_fs_info        = fsi;
 	fc->ops              = &famfs_context_ops;
@@ -251,6 +483,8 @@ static void famfs_kill_sb(struct super_block *sb)
 {
 	struct famfs_fs_info *fsi = sb->s_fs_info;
 
+	famfs_devlist_free(fsi, sb);
+
 	kill_char_super(sb);
 
 	kfree(fsi);
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index 6f481d79ed88..ebb9c499cf69 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -11,22 +11,68 @@
 #ifndef FAMFS_INTERNAL_H
 #define FAMFS_INTERNAL_H
 
+#include <linux/rwsem.h>
+#include <linux/bits.h>
+#include <linux/build_bug.h>
+
 struct famfs_mount_opts {
 	umode_t mode;
 };
 
+/*
+ * famfs_daxdev - one entry in the per-superblock daxdev table
+ *
+ * @valid:   slot is populated and the daxdev has been exclusively acquired
+ * @error:   dax reported a memory error (probably poison) via notify_failure
+ * @dax_err: fs_dax_get() failed for this daxdev
+ * @devno:   dax device dev_t
+ * @devp:    the acquired dax_device
+ * @name:    dax device path (may be NULL for ioctl-registered daxdevs)
+ */
+struct famfs_daxdev {
+	bool valid;
+	bool error;
+	bool dax_err;
+	dev_t devno;
+	struct dax_device *devp;
+	char *name;
+};
+
+/*
+ * The daxdev index space (and thus this table) is capped at 64 so the set of
+ * daxdev indices referenced by a file's fmap fits in a u64 bitmap.
+ */
+#define FAMFS_MAX_DAXDEVS 64
+static_assert(BITS_PER_TYPE(u64) >= FAMFS_MAX_DAXDEVS);
+
+/*
+ * famfs_dax_devlist - the per-superblock table of famfs_daxdev's. Slot 0 is
+ * the primary daxdev supplied at mount; slots 1..n are registered via ioctl.
+ */
+struct famfs_dax_devlist {
+	int nslots;
+	struct famfs_daxdev *devlist;
+};
+
 /**
  * @famfs_fs_info
  *
  * @mount_opts:  The mount options
  * @deverror:    True if the dax device has called our notify_failure entry
  *               point, or if other "shutdown" conditions exist
+ * @dax_devlist: Table of backing daxdevs (slot 0 is the mount primary)
+ * @devlist_sem: Serializes installs into, and teardown of, @dax_devlist
  */
 struct famfs_fs_info {
 	struct famfs_mount_opts   mount_opts;
 	bool                      deverror;
+	struct famfs_dax_devlist *dax_devlist;
+	struct rw_semaphore       devlist_sem;
 };
 
 int lookup_daxdev(const char *pathname, dev_t *devno);
+int famfs_devlist_alloc(struct famfs_fs_info *fsi);
+int famfs_install_daxdev(struct famfs_fs_info *fsi, struct super_block *sb,
+			 u64 index, dev_t devno, const char *name);
 
 #endif /* FAMFS_INTERNAL_H */
-- 
2.53.0

Re: [PATCH V12 03/12] famfs: Add daxdev table and dax notify_failure support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition in `famfs_dax_notify_failure` causes NULL pointer dereference or Use-After-Free during teardown.
- [Medium] famfs_install_daxdev silently returns success when a conflicting device is installed at an already-populated index.
--

--- Patch [3]: [PATCH V12 03/12] famfs: Add daxdev table and dax notify_failure support ---
commit 6ec60105b0cea4058f8c078b72261124109aca1d
Author: John Groves <John@Groves.net>

famfs: Add daxdev table and dax notify_failure support

Famfs file systems can span multiple dax devices, and daxdevs are stored
in the daxdev_table. This adds the basic table structure, primtives and
serialization code. Famfs file extents reference daxdevs by index, which
is a cluster invariant maintained by user space.

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index c299a90912a5c..ad71e5e7a8e39 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -75,6 +75,225 @@ static struct inode *famfs_get_inode(
 /*
  * famfs dax_operations (for famfs-mode dax)
  */
[ ... ]
> +static int
> +famfs_dax_notify_failure(
> +		struct dax_device *dax_dev, u64 offset,
> +		u64 len, int mf_flags)
> +{
> +	struct super_block *sb = dax_holder(dax_dev);
> +	struct famfs_fs_info *fsi = sb->s_fs_info;

[Severity: High]
Can this race with a concurrent filesystem unmount?

If a memory failure triggers famfs_dax_notify_failure() concurrently with
famfs_kill_sb(), fs_put_dax() may clear the holder data asynchronously. If
dax_holder() returns NULL here, does this result in a NULL pointer dereference
when accessing sb->s_fs_info?

Furthermore, if dax_holder() fetches the superblock just before it is cleared:

CPU1
famfs_dax_notify_failure()
    sb = dax_holder(dax_dev);
    fsi = sb->s_fs_info;

CPU2
famfs_kill_sb()
    famfs_devlist_free(fsi, sb);
    kill_char_super(sb);
    kfree(fsi);

CPU1
    famfs_set_daxdev_err(fsi, dax_dev);

Could this result in a use-after-free when famfs_set_daxdev_err() attempts
to acquire the freed fsi->devlist_sem?

[ ... ]
> +int famfs_install_daxdev(
> +		struct famfs_fs_info *fsi,
> +		struct super_block *sb,
> +		u64 index,
> +		dev_t devno,
> +		const char *name)
> +{
[ ... ]
> +	scoped_guard(rwsem_write, &fsi->devlist_sem) {
> +		daxdev = &fsi->dax_devlist->devlist[index];
> +
> +		/* Installed already by a concurrent (or repeated) open */
> +		if (daxdev->valid)
> +			return 0;

[Severity: Medium]
If userspace erroneously requests mapping a new or different dax device to an
index that is already populated, this path silently returns 0.

Should this verify that the requested devno matches the already installed
device's devno, and return an error like -EBUSY or -EEXIST if it does not, to
prevent misleading userspace about which mapping is actually active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=3

[PATCH V12 04/12] famfs: Introduce inode_operations and super_operations

[John Groves]

From: John Groves <john@groves.net>

The famfs inode and super operations are generic other than
show_options, evict_inode and setattr (which prevents truncation..

This commit builds but is still too incomplete to run

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_inode.c    | 249 +++++++++++++++++++++++++++++++++++++-
 fs/famfs/famfs_internal.h |   6 +
 2 files changed, 252 insertions(+), 3 deletions(-)

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index ad71e5e7a8e3..efc6b852eca0 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -29,6 +29,9 @@
 
 #define FAMFS_DEFAULT_MODE	0755
 
+static const struct inode_operations famfs_file_inode_operations;
+static const struct inode_operations famfs_dir_inode_operations;
+
 static struct inode *famfs_get_inode(
 			struct super_block *sb,
 			const struct inode *dir,
@@ -54,11 +57,11 @@ static struct inode *famfs_get_inode(
 		init_special_inode(inode, mode, dev);
 		break;
 	case S_IFREG:
-		inode->i_op = NULL /* famfs_file_inode_operations */;
+		inode->i_op = &famfs_file_inode_operations;
 		inode->i_fop = NULL /* &famfs_file_operations */;
 		break;
 	case S_IFDIR:
-		inode->i_op = NULL /* famfs_dir_inode_operations */;
+		inode->i_op = &famfs_dir_inode_operations;
 		inode->i_fop = &simple_dir_operations;
 
 		/* Directory inodes start off with i_nlink == 2 (for ".") */
@@ -72,6 +75,246 @@ static struct inode *famfs_get_inode(
 	return inode;
 }
 
+/***************************************************************************
+ * famfs inode_operations
+ */
+
+static int
+famfs_setattr(
+	struct mnt_idmap *idmap,
+	struct dentry *dentry,
+	struct iattr *iattr)
+{
+	struct inode *inode = d_inode(dentry);
+	struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
+
+	/* Resizing a famfs file (its size is pinned to the fmap) */
+	if ((iattr->ia_valid & ATTR_SIZE) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_TRUNCATE) &&
+	    iattr->ia_size != i_size_read(inode))
+		return -EPERM;
+	if ((iattr->ia_valid & ATTR_MODE) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_CHMOD))
+		return -EPERM;
+	if ((iattr->ia_valid & (ATTR_UID | ATTR_GID)) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_CHOWN))
+		return -EPERM;
+	if ((iattr->ia_valid & (ATTR_ATIME | ATTR_MTIME)) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_UTIMES))
+		return -EPERM;
+
+	return simple_setattr(idmap, dentry, iattr);
+}
+
+static const struct inode_operations famfs_file_inode_operations = {
+	/* All generic */
+	.setattr	   = famfs_setattr,
+	.getattr	   = simple_getattr,
+};
+
+/*
+ * Internal inode creation helper, shared by ->create, ->mkdir, ->mknod and
+ * ->symlink. Each of those callers is responsible for its own FAMFS_OPT_*
+ * permission check before getting here.
+ */
+static int
+famfs_mknod(struct mnt_idmap *idmap, struct inode *dir, struct dentry *dentry,
+	    umode_t mode, dev_t dev)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+	struct timespec64 tv;
+	struct inode *inode;
+
+	if (fsi->deverror)
+		return -ENODEV;
+
+	inode = famfs_get_inode(dir->i_sb, dir, mode, dev);
+	if (!inode)
+		return -ENOSPC;
+
+	d_make_persistent(dentry, inode);
+	tv = inode_set_ctime_current(inode);
+	inode_set_mtime_to_ts(inode, tv);
+	inode_set_atime_to_ts(inode, tv);
+
+	return 0;
+}
+
+static struct dentry *famfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
+		struct dentry *dentry, umode_t mode)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+	int rc;
+
+	if (fsi->deverror)
+		return ERR_PTR(-ENODEV);
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_MKDIR))
+		return ERR_PTR(-EPERM);
+
+	rc = famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFDIR, 0);
+	if (rc)
+		return ERR_PTR(rc);
+
+	inc_nlink(dir);
+
+	return ERR_PTR(0);
+}
+
+static int famfs_create(struct mnt_idmap *idmap, struct inode *dir,
+			struct dentry *dentry, umode_t mode, bool excl)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+
+	if (fsi->deverror)
+		return -ENODEV;
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_CREATE))
+		return -EPERM;
+
+	return famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFREG, 0);
+}
+
+static int
+famfs_mknod_op(struct mnt_idmap *idmap, struct inode *dir,
+	       struct dentry *dentry, umode_t mode, dev_t dev)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_MKNOD))
+		return -EPERM;
+
+	return famfs_mknod(idmap, dir, dentry, mode, dev);
+}
+
+static int
+famfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
+	      struct dentry *dentry, const char *symname)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+	struct inode *inode;
+	int len, rc;
+
+	if (fsi->deverror)
+		return -ENODEV;
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_SYMLINK))
+		return -EPERM;
+
+	inode = famfs_get_inode(dir->i_sb, dir, S_IFLNK | 0777, 0);
+	if (!inode)
+		return -ENOSPC;
+
+	len = strlen(symname) + 1;
+	rc = page_symlink(inode, symname, len);
+	if (rc) {
+		iput(inode);
+		return rc;
+	}
+
+	d_make_persistent(dentry, inode);
+	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
+
+	return 0;
+}
+
+static int
+famfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_LINK))
+		return -EPERM;
+
+	return simple_link(old_dentry, dir, dentry);
+}
+
+static int famfs_unlink(struct inode *dir, struct dentry *dentry)
+{
+	struct inode *inode = d_inode(dentry);
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+
+	/* A file with an fmap may only be unlinked when explicitly enabled */
+	if (inode->i_private && !famfs_opt_enabled(fsi, FAMFS_OPT_UNLINK))
+		return -EPERM;
+
+	return simple_unlink(dir, dentry);
+}
+
+static int famfs_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_RMDIR))
+		return -EPERM;
+
+	return simple_rmdir(dir, dentry);
+}
+
+static int
+famfs_rename(
+	struct mnt_idmap *idmap,
+	struct inode *old_dir,
+	struct dentry *old_dentry,
+	struct inode *new_dir,
+	struct dentry *new_dentry,
+	unsigned int flags)
+{
+	struct famfs_fs_info *fsi = old_dir->i_sb->s_fs_info;
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_RENAME))
+		return -EPERM;
+
+	return simple_rename(idmap, old_dir, old_dentry, new_dir, new_dentry,
+			     flags);
+}
+
+static const struct inode_operations famfs_dir_inode_operations = {
+	.create		= famfs_create,
+	.lookup		= simple_lookup,
+	.link		= famfs_link,
+	.unlink		= famfs_unlink,
+	.symlink	= famfs_symlink,
+	.mkdir		= famfs_mkdir,
+	.mknod		= famfs_mknod_op,
+	.rmdir		= famfs_rmdir,
+	.rename		= famfs_rename,
+};
+
+/*****************************************************************************
+ * famfs super_operations
+ *
+ * TODO: implement a famfs_statfs() that shows size, free and available space,
+ * etc.
+ */
+
+/*
+ * famfs_show_options() - Display the mount options in /proc/mounts.
+ */
+static int famfs_show_options(struct seq_file *m, struct dentry *root)
+{
+	struct famfs_fs_info *fsi = root->d_sb->s_fs_info;
+
+	if (fsi->mount_opts.mode != FAMFS_DEFAULT_MODE)
+		seq_printf(m, ",mode=%o", fsi->mount_opts.mode);
+
+	return 0;
+}
+
+static void famfs_evict_inode(struct inode *inode)
+{
+	inode->i_private = NULL;
+	dax_break_layout_final(inode);
+	truncate_inode_pages_final(&inode->i_data);
+	clear_inode(inode);
+}
+
+static const struct super_operations famfs_super_ops = {
+	.statfs		= simple_statfs,
+	.drop_inode	= inode_just_drop,
+	.show_options	= famfs_show_options,
+	.evict_inode    = famfs_evict_inode,
+};
+
+/*****************************************************************************/
+
 /*
  * famfs dax_operations (for famfs-mode dax)
  */
@@ -305,7 +548,7 @@ famfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_blocksize		= PAGE_SIZE;
 	sb->s_blocksize_bits	= PAGE_SHIFT;
 	sb->s_magic		= FAMFS_SUPER_MAGIC;
-	sb->s_op		= NULL /* famfs_super_ops */;
+	sb->s_op		= &famfs_super_ops;
 	sb->s_time_gran		= 1;
 }
 
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index ebb9c499cf69..485087588a11 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -70,6 +70,12 @@ struct famfs_fs_info {
 	struct rw_semaphore       devlist_sem;
 };
 
+/* This stub will be replaced in a later commit 
+ * Note: the opt parameter is intentionally unused, and will be used by
+ * the replacement function when that commit lands
+ */
+#define famfs_opt_enabled(fsi, opt) (fsi != 0)
+
 int lookup_daxdev(const char *pathname, dev_t *devno);
 int famfs_devlist_alloc(struct famfs_fs_info *fsi);
 int famfs_install_daxdev(struct famfs_fs_info *fsi, struct super_block *sb,
-- 
2.53.0

Re: [PATCH V12 04/12] famfs: Introduce inode_operations and super_operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Setting `inode->i_fop` to `NULL` for regular files triggers a kernel WARN_ON during `open()`, leading to a denial of service.
- [High] The `FAMFS_OPT_UNLINK` and `FAMFS_OPT_RMDIR` policies can be trivially bypassed by renaming an entry over existing files or directories.
- [Medium] Creation operations (create, mkdir, mknod) fail to update the parent directory's timestamps, and racily update the child inode's timestamps after publishing.
--

--- Patch [4]: [PATCH V12 04/12] famfs: Introduce inode_operations and super_operations ---
commit 1062e22926010cfce092f1770e05033249b3ab2f
Author: John Groves <john@groves.net>

famfs: Introduce inode_operations and super_operations

The famfs inode and super operations are generic other than
show_options, evict_inode and setattr. This commit builds but is
still too incomplete to run.

> diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
> index ad71e5e7a8e39..efc6b852eca00 100644
> --- a/fs/famfs/famfs_inode.c
> +++ b/fs/famfs/famfs_inode.c
[ ... ]
> @@ -54,11 +57,11 @@ static struct inode *famfs_get_inode(
>  		init_special_inode(inode, mode, dev);
>  		break;
>  	case S_IFREG:
> -		inode->i_op = NULL /* famfs_file_inode_operations */;
> +		inode->i_op = &famfs_file_inode_operations;
>  		inode->i_fop = NULL /* &famfs_file_operations */;

[Severity: High]
Will this trigger a kernel warning during open()?

When VFS attempts to open a regular file, do_dentry_open() calls
fops_get(inode->i_fop). Passing a NULL f_op pointer triggers a
WARN_ON(!f->f_op), which could lead to a denial of service on systems
with panic_on_warn enabled.

>  		break;
>  	case S_IFDIR:
> -		inode->i_op = NULL /* famfs_dir_inode_operations */;
> +		inode->i_op = &famfs_dir_inode_operations;
>  		inode->i_fop = &simple_dir_operations;
[ ... ]
> +static int
> +famfs_mknod(struct mnt_idmap *idmap, struct inode *dir, struct dentry *dentry,
> +	    umode_t mode, dev_t dev)
> +{
> +	struct famfs_fs_info *fsi = dir->i_sb->s_fs_info;
> +	struct timespec64 tv;
> +	struct inode *inode;
> +
> +	if (fsi->deverror)
> +		return -ENODEV;
> +
> +	inode = famfs_get_inode(dir->i_sb, dir, mode, dev);
> +	if (!inode)
> +		return -ENOSPC;
> +
> +	d_make_persistent(dentry, inode);
> +	tv = inode_set_ctime_current(inode);
> +	inode_set_mtime_to_ts(inode, tv);
> +	inode_set_atime_to_ts(inode, tv);

[Severity: Medium]
Is there a typo here with 'inode' instead of 'dir'?

Creation operations should update the parent directory's mtime and ctime.
Updating the newly created child inode's timestamps here seems redundant since
they are already initialized in famfs_get_inode(). 

Additionally, modifying the child inode's timestamps after calling
d_make_persistent() publishes it to the dcache creates a race condition
against lockless lookups.

> +
> +	return 0;
> +}
[ ... ]
> +static int
> +famfs_rename(
> +	struct mnt_idmap *idmap,
> +	struct inode *old_dir,
> +	struct dentry *old_dentry,
> +	struct inode *new_dir,
> +	struct dentry *new_dentry,
> +	unsigned int flags)
> +{
> +	struct famfs_fs_info *fsi = old_dir->i_sb->s_fs_info;
> +
> +	if (!famfs_opt_enabled(fsi, FAMFS_OPT_RENAME))
> +		return -EPERM;
> +
> +	return simple_rename(idmap, old_dir, old_dentry, new_dir, new_dentry,
> +			     flags);

[Severity: High]
Can this implementation bypass the protections in famfs_unlink() and
famfs_rmdir()?

The filesystem implements safety checks in famfs_unlink() to prevent
the removal of files with active memory maps (where inode->i_private is
set). However, delegating directly to simple_rename() will internally call
simple_unlink() or simple_rmdir() on an existing target without invoking the
custom policy checks.

Could this allow a policy bypass by renaming an entry over a protected
target, potentially destroying active shared memory states?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=4

[PATCH V12 05/12] famfs: Introduce file_operations read/write

[John Groves]

From: John Groves <john@groves.net>

This commit introduces fs/famfs/famfs_file.c and the famfs
file_operations for read/write.

This is not usable yet because:

* It calls dax_iomap_rw() with NULL iomap_ops (which will be
  introduced in a subsequent commit).
* famfs_ioctl() is coming in a later commit, and it is necessary
  to map a file to a memory allocation.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/Makefile         |   2 +-
 fs/famfs/famfs_file.c     | 138 ++++++++++++++++++++++++++++++++++++++
 fs/famfs/famfs_inode.c    |   2 +-
 fs/famfs/famfs_internal.h |   2 +
 4 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 fs/famfs/famfs_file.c

diff --git a/fs/famfs/Makefile b/fs/famfs/Makefile
index 62230bcd6793..8cac90c090a4 100644
--- a/fs/famfs/Makefile
+++ b/fs/famfs/Makefile
@@ -2,4 +2,4 @@
 
 obj-$(CONFIG_FAMFS) += famfs.o
 
-famfs-y := famfs_inode.o
+famfs-y := famfs_inode.o famfs_file.o
diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
new file mode 100644
index 000000000000..e192b573c51f
--- /dev/null
+++ b/fs/famfs/famfs_file.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * famfs - dax file system for shared fabric-attached memory
+ *
+ * Copyright 2023-2024 Micron Technology, Inc.
+ *
+ * This file system, originally based on ramfs the dax support from xfs,
+ * is intended to allow multiple host systems to mount a common file system
+ * view of dax files that map to shared memory.
+ */
+
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/dax.h>
+#include <linux/iomap.h>
+
+#include "famfs_internal.h"
+
+/*********************************************************************
+ * file_operations
+ */
+
+/* Reject I/O to files that aren't in a valid state */
+static ssize_t
+famfs_file_invalid(struct inode *inode)
+{
+	if (!IS_DAX(inode)) {
+		pr_debug("%s: inode %llx IS_DAX is false\n",
+			 __func__, (u64)inode);
+		return -ENXIO;
+	}
+	return 0;
+}
+
+static ssize_t
+famfs_rw_prep(struct kiocb *iocb, struct iov_iter *ubuf)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	struct super_block *sb = inode->i_sb;
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+	size_t i_size = i_size_read(inode);
+	size_t count = iov_iter_count(ubuf);
+	size_t max_count;
+	ssize_t rc;
+
+	if (fsi->deverror)
+		return -ENODEV;
+
+	rc = famfs_file_invalid(inode);
+	if (rc)
+		return rc;
+
+	/* Avoid unsigned underflow if position is past EOF */
+	if (iocb->ki_pos >= i_size)
+		max_count = 0;
+	else
+		max_count = i_size - iocb->ki_pos;
+
+	if (count > max_count)
+		iov_iter_truncate(ubuf, max_count);
+
+	if (!iov_iter_count(ubuf))
+		return 0;
+
+	return rc;
+}
+
+static ssize_t
+famfs_dax_read_iter(struct kiocb *iocb, struct iov_iter	*to)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	ssize_t rc;
+
+	/* dax_iomap_rw() requires i_rwsem held (shared for read) */
+	inode_lock_shared(inode);
+	rc = famfs_rw_prep(iocb, to);
+	if (rc || !iov_iter_count(to)) {
+		inode_unlock_shared(inode);
+		return rc;
+	}
+
+	rc = dax_iomap_rw(iocb, to, NULL /*&famfs_iomap_ops */);
+	inode_unlock_shared(inode);
+
+	file_accessed(iocb->ki_filp);
+	return rc;
+}
+
+/**
+ * famfs_dax_write_iter()
+ *
+ * We need our own write-iter in order to prevent append
+ *
+ * @iocb:
+ * @from: iterator describing the user memory source for the write
+ */
+static ssize_t
+famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
+	ssize_t rc;
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))
+		return -EPERM;
+
+	/* dax_iomap_rw() requires i_rwsem held (exclusive for write) */
+	inode_lock(inode);
+	rc = famfs_rw_prep(iocb, from);
+	if (rc || !iov_iter_count(from)) {
+		inode_unlock(inode);
+		return rc;
+	}
+
+	rc = dax_iomap_rw(iocb, from, NULL /*&famfs_iomap_ops*/);
+	inode_unlock(inode);
+	return rc;
+}
+
+const struct file_operations famfs_file_operations = {
+	.owner             = THIS_MODULE,
+
+	/* Custom famfs operations */
+	.write_iter	   = famfs_dax_write_iter,
+	.read_iter	   = famfs_dax_read_iter,
+	.unlocked_ioctl    = NULL /*famfs_file_ioctl*/,
+	.mmap		   = NULL /* famfs_file_mmap */,
+
+	/* Force PMD alignment for mmap */
+	.get_unmapped_area = thp_get_unmapped_area,
+
+	/* Generic Operations */
+	.fsync		   = noop_fsync,
+	.splice_read	   = filemap_splice_read,
+	.splice_write	   = iter_file_splice_write,
+	.llseek		   = generic_file_llseek,
+};
+
diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index efc6b852eca0..910a143dad30 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -58,7 +58,7 @@ static struct inode *famfs_get_inode(
 		break;
 	case S_IFREG:
 		inode->i_op = &famfs_file_inode_operations;
-		inode->i_fop = NULL /* &famfs_file_operations */;
+		inode->i_fop = &famfs_file_operations;
 		break;
 	case S_IFDIR:
 		inode->i_op = &famfs_dir_inode_operations;
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index 485087588a11..26f5abda96dc 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -15,6 +15,8 @@
 #include <linux/bits.h>
 #include <linux/build_bug.h>
 
+extern const struct file_operations famfs_file_operations;
+
 struct famfs_mount_opts {
 	umode_t mode;
 };
-- 
2.53.0

Re: [PATCH V12 05/12] famfs: Introduce file_operations read/write

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] The custom write iterator famfs_dax_write_iter() omits the required VFS file_modified() / file_remove_privs() hooks, bypassing setuid/setgid privilege stripping and timestamp updates.
- [High] famfs_dax_write_iter() bypasses POSIX append semantics and file size limits by omitting generic_write_checks().
- [High] famfs_rw_prep() truncates the 64-bit file size to 32 bits on 32-bit architectures, breaking bounds checking and causing artificial IO limits.
- [High] Using filemap_splice_read on a DAX filesystem initialized with ram_aops will cause splice reads to silently return streams of zeroes.
- [Medium] Writes past the end of the file return 0 instead of an error, which can cause infinite loops in userspace.
- [Low] file_accessed() is called unconditionally on reads, updating access time even on errors or EOF.
- [Low] Kernel heap address leak via pr_debug using %llx on an inode pointer.
--

--- Patch [5]: [PATCH V12 05/12] famfs: Introduce file_operations read/write ---
commit 025e683c34185b254d9bc60d68aabd4be0342a87
Author: John Groves <john@groves.net>

famfs: Introduce file_operations read/write

This commit introduces fs/famfs/famfs_file.c and the famfs
file_operations for read/write. It sets up the groundwork for
DAX I/O, though it currently calls dax_iomap_rw with NULL iomap_ops.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> new file mode 100644
> index 0000000000000..e192b573c51f7
> --- /dev/null
> +++ b/fs/famfs/famfs_file.c
> @@ -0,0 +1,138 @@
[ ... ]
> +/* Reject I/O to files that aren't in a valid state */
> +static ssize_t
> +famfs_file_invalid(struct inode *inode)
> +{
> +	if (!IS_DAX(inode)) {
> +		pr_debug("%s: inode %llx IS_DAX is false\n",
> +			 __func__, (u64)inode);

[Severity: Low]
Does this explicit cast to u64 leak the raw kernel heap address of the inode?

When dynamic debugging is enabled and famfs_file_invalid() is called on an
inode without S_DAX, bypassing the kernel's %p hashing protection could expose
the heap layout to dmesg. Could this assist attackers in bypassing KASLR?

> +		return -ENXIO;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t
> +famfs_rw_prep(struct kiocb *iocb, struct iov_iter *ubuf)
> +{
> +	struct inode *inode = iocb->ki_filp->f_mapping->host;
> +	struct super_block *sb = inode->i_sb;
> +	struct famfs_fs_info *fsi = sb->s_fs_info;
> +	size_t i_size = i_size_read(inode);

[Severity: High]
Does this truncate the 64-bit loff_t file size to 32 bits on 32-bit
architectures?

When reading or writing a famfs file larger than 4GB on a 32-bit system,
i_size_read() will be truncated into the 32-bit size_t i_size. Will this break
bounds checking and make data past the 32-bit wrapped boundary inaccessible?

> +	size_t count = iov_iter_count(ubuf);
> +	size_t max_count;
> +	ssize_t rc;
[ ... ]
> +	/* Avoid unsigned underflow if position is past EOF */
> +	if (iocb->ki_pos >= i_size)
> +		max_count = 0;
> +	else
> +		max_count = i_size - iocb->ki_pos;
> +
> +	if (count > max_count)
> +		iov_iter_truncate(ubuf, max_count);
> +
> +	if (!iov_iter_count(ubuf))
> +		return 0;

[Severity: Medium]
When writing at or past EOF (ki_pos >= i_size), famfs_rw_prep() sets max_count
to 0 and returns 0. Will famfs_dax_write_iter() bubble this 0 up to the
syscall instead of returning an error code like -ENOSPC or -EFBIG?

Userspace tools often interpret a 0-byte write return as a temporary
condition and retry indefinitely. Could this cause unprivileged applications
to hang in infinite loops?

> +
> +	return rc;
> +}
> +
> +static ssize_t
> +famfs_dax_read_iter(struct kiocb *iocb, struct iov_iter	*to)
> +{
[ ... ]
> +	rc = dax_iomap_rw(iocb, to, NULL /*&famfs_iomap_ops */);
> +	inode_unlock_shared(inode);
> +
> +	file_accessed(iocb->ki_filp);

[Severity: Low]
Should file_accessed() only be called if the read successfully transferred
data (rc > 0)?

Unconditionally calling file_accessed() in famfs_dax_read_iter() will update
access times and generate metadata traffic even for failed reads or zero-byte
EOF reads.

> +	return rc;
> +}
> +
> +/**
> + * famfs_dax_write_iter()
> + *
> + * We need our own write-iter in order to prevent append
> + *
> + * @iocb:
> + * @from: iterator describing the user memory source for the write
> + */
> +static ssize_t
> +famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	struct inode *inode = iocb->ki_filp->f_mapping->host;
> +	struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
> +	ssize_t rc;
> +
> +	if (!famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))
> +		return -EPERM;
> +
> +	/* dax_iomap_rw() requires i_rwsem held (exclusive for write) */
> +	inode_lock(inode);
> +	rc = famfs_rw_prep(iocb, from);

[Severity: High]
Does famfs_dax_write_iter() bypass POSIX append semantics and file size limits
by omitting generic_write_checks() here?

Without generic_write_checks() before preparing the IO, any user process
opening a file with O_APPEND or a restricted RLIMIT_FSIZE can write directly
to the current offset and bypass those limits.

> +	if (rc || !iov_iter_count(from)) {
> +		inode_unlock(inode);
> +		return rc;
> +	}
> +
> +	rc = dax_iomap_rw(iocb, from, NULL /*&famfs_iomap_ops*/);

[Severity: High]
Does this bypass setuid/setgid privilege stripping and timestamp updates?

The famfs_dax_write_iter() function modifies the file via dax_iomap_rw()
without calling the required VFS hooks like file_modified() or
file_remove_privs(). Could this omission allow unprivileged users to escalate
privileges on SUID/SGID files they have write access to?

> +	inode_unlock(inode);
> +	return rc;
> +}
> +
> +const struct file_operations famfs_file_operations = {
> +	.owner             = THIS_MODULE,
> +
[ ... ]
> +	/* Generic Operations */
> +	.fsync		   = noop_fsync,
> +	.splice_read	   = filemap_splice_read,

[Severity: High]
Is filemap_splice_read the correct operation for a DAX filesystem?

Because famfs inodes lack the S_DAX flag, VFS falls back to using
filemap_splice_read. This function queries the page cache and allocates new
pages using ram_aops.read_folio (which resolves to simple_read_folio and zero
fills them). Will userspace applications using splice() or sendfile() receive
a stream of zeroes instead of the actual DAX memory contents?

> +	.splice_write	   = iter_file_splice_write,
> +	.llseek		   = generic_file_llseek,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=5

[PATCH V12 06/12] famfs: Introduce mmap and VM fault handling

[John Groves]

From: John Groves <john@groves.net>

This commit adds vm_operations, plus famfs_mmap() and fault handlers.
It is still missing iomap_ops, iomap mapping resolution, and
famfs_ioctl() for setting up file-to-memory mappings.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_file.c | 101 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 1 deletion(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index e192b573c51f..678f2035fd5f 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -16,6 +16,75 @@
 
 #include "famfs_internal.h"
 
+/*********************************************************************
+ * vm_operations
+ */
+static vm_fault_t
+__famfs_filemap_fault(struct vm_fault *vmf, unsigned int order,
+		      bool write_fault)
+{
+	struct inode *inode = file_inode(vmf->vma->vm_file);
+	struct super_block *sb = inode->i_sb;
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+	vm_fault_t ret;
+	unsigned long pfn;
+
+	if (fsi->deverror)
+		return VM_FAULT_SIGBUS;
+
+	if (!IS_DAX(file_inode(vmf->vma->vm_file))) {
+		pr_err("%s: file not marked IS_DAX!!\n", __func__);
+		return VM_FAULT_SIGBUS;
+	}
+
+	if (write_fault) {
+		sb_start_pagefault(inode->i_sb);
+		file_update_time(vmf->vma->vm_file);
+	}
+
+	ret = dax_iomap_fault(vmf, order, &pfn, NULL, NULL /*&famfs_iomap_ops */);
+	if (ret & VM_FAULT_NEEDDSYNC)
+		ret = dax_finish_sync_fault(vmf, order, pfn);
+
+	if (write_fault)
+		sb_end_pagefault(inode->i_sb);
+
+	return ret;
+}
+
+static inline bool
+famfs_is_write_fault(struct vm_fault *vmf)
+{
+	return (vmf->flags & FAULT_FLAG_WRITE) &&
+	       (vmf->vma->vm_flags & VM_SHARED);
+}
+
+static vm_fault_t
+famfs_filemap_fault(struct vm_fault *vmf)
+{
+	return __famfs_filemap_fault(vmf, 0, famfs_is_write_fault(vmf));
+}
+
+static vm_fault_t
+famfs_filemap_huge_fault(struct vm_fault *vmf, unsigned int order)
+{
+	return __famfs_filemap_fault(vmf, order, famfs_is_write_fault(vmf));
+}
+
+static vm_fault_t
+famfs_filemap_mkwrite(struct vm_fault *vmf)
+{
+	return __famfs_filemap_fault(vmf, 0, true);
+}
+
+const struct vm_operations_struct famfs_file_vm_ops = {
+	.fault		= famfs_filemap_fault,
+	.huge_fault	= famfs_filemap_huge_fault,
+	.map_pages	= filemap_map_pages,
+	.page_mkwrite	= famfs_filemap_mkwrite,
+	.pfn_mkwrite	= famfs_filemap_mkwrite,
+};
+
 /*********************************************************************
  * file_operations
  */
@@ -117,6 +186,36 @@ famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	return rc;
 }
 
+static int
+famfs_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct inode *inode = file_inode(file);
+	struct super_block *sb = inode->i_sb;
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+	ssize_t rc;
+
+	if (fsi->deverror)
+		return -ENODEV;
+
+	/*
+	 * Gate shared-writable mappings on FAMFS_OPT_WRITE. This is best
+	 * effort: clearing the bit blocks new writable mappings and write(),
+	 * but does not revoke mappings that already exist.
+	 */
+	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))
+		return -EPERM;
+
+	rc = famfs_file_invalid(inode);
+	if (rc)
+		return (int)rc;
+
+	file_accessed(file);
+	vma->vm_ops = &famfs_file_vm_ops;
+	vm_flags_set(vma, VM_HUGEPAGE);
+	return 0;
+}
+
 const struct file_operations famfs_file_operations = {
 	.owner             = THIS_MODULE,
 
@@ -124,7 +223,7 @@ const struct file_operations famfs_file_operations = {
 	.write_iter	   = famfs_dax_write_iter,
 	.read_iter	   = famfs_dax_read_iter,
 	.unlocked_ioctl    = NULL /*famfs_file_ioctl*/,
-	.mmap		   = NULL /* famfs_file_mmap */,
+	.mmap		   = famfs_file_mmap,
 
 	/* Force PMD alignment for mmap */
 	.get_unmapped_area = thp_get_unmapped_area,
-- 
2.53.0

Re: [PATCH V12 06/12] famfs: Introduce mmap and VM fault handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Medium] Hardcoded NULL iomap_ops passed to DAX functions will cause a kernel panic
- [Medium] Missing file_modified() call allows bypassing security privileges on write
- [Medium] Missing generic_write_sync() silently breaks DAX synchronous writes
- [Medium] Missing FOP_MMAP_SYNC flag prevents MAP_SYNC usage
- [Medium] mprotect() can bypass the FAMFS_OPT_WRITE gate to gain writable mappings
- [Medium] Missing lock serialization between DAX page faults and file truncation
- [Low] Setting .map_pages to filemap_map_pages is invalid for DAX mappings
--

--- Patch [6]: [PATCH V12 06/12] famfs: Introduce mmap and VM fault handling ---
commit a41970b8d49174c2162aaa9777d2fc2abc414431
Author: John Groves <john@groves.net>

famfs: Introduce mmap and VM fault handling

This commit adds vm_operations, famfs_mmap(), and fault handlers to the
famfs filesystem. It sets up the groundwork for memory mapping DAX files,
though it is still missing iomap_ops and mapping resolution.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index e192b573c51f7..678f2035fd5fa 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
> @@ -16,6 +16,75 @@
>  
>  #include "famfs_internal.h"
>  
> +/*********************************************************************
> + * vm_operations
> + */
> +static vm_fault_t
> +__famfs_filemap_fault(struct vm_fault *vmf, unsigned int order,
> +		      bool write_fault)
> +{
> +	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	struct super_block *sb = inode->i_sb;
> +	struct famfs_fs_info *fsi = sb->s_fs_info;
> +	vm_fault_t ret;
> +	unsigned long pfn;
> +
> +	if (fsi->deverror)
> +		return VM_FAULT_SIGBUS;
> +
> +	if (!IS_DAX(file_inode(vmf->vma->vm_file))) {
> +		pr_err("%s: file not marked IS_DAX!!\n", __func__);
> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	if (write_fault) {
> +		sb_start_pagefault(inode->i_sb);
> +		file_update_time(vmf->vma->vm_file);
> +	}
> +
> +	ret = dax_iomap_fault(vmf, order, &pfn, NULL, NULL /*&famfs_iomap_ops */);

[Severity: Medium]
Will this hardcoded NULL pointer for the iomap_ops argument cause a kernel
panic? The core DAX subsystem unconditionally dereferences the ops pointer
(for example, by calling ops->iomap_begin()). This also appears to happen
in famfs_dax_read_iter() and famfs_dax_write_iter() when calling
dax_iomap_rw().

[Severity: Medium]
Is there missing lock serialization between DAX page faults and file
truncation here? Calling the DAX page fault handler without holding an
internal filesystem lock (like i_mmap_sem) might allow concurrent truncation
and page faults to race, mapping DAX pages out-of-bounds or into truncated
regions.

> +	if (ret & VM_FAULT_NEEDDSYNC)
> +		ret = dax_finish_sync_fault(vmf, order, pfn);
> +
> +	if (write_fault)
> +		sb_end_pagefault(inode->i_sb);
> +
> +	return ret;
> +}
> +
[ ... ]
> +const struct vm_operations_struct famfs_file_vm_ops = {
> +	.fault		= famfs_filemap_fault,
> +	.huge_fault	= famfs_filemap_huge_fault,
> +	.map_pages	= filemap_map_pages,

[Severity: Low]
Is it valid to assign filemap_map_pages to a DAX filesystem?
filemap_map_pages is designed to map standard page cache folios, but DAX
filesystems use value entries in the xarray. Since modern kernels safely
skip these via xa_is_value(), iterating the xarray to map nothing wastes
CPU cycles and appears to be an architectural violation.

> +	.page_mkwrite	= famfs_filemap_mkwrite,
> +	.pfn_mkwrite	= famfs_filemap_mkwrite,
> +};
> +
>  /*********************************************************************
>   * file_operations
>   */
> @@ -117,6 +186,36 @@ famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)

[Severity: Medium]
Does the I/O performed in famfs_dax_write_iter() miss a preceding call to
file_modified()? If so, when an unprivileged user writes to a file, the
setuid and setgid bits are not stripped, and the mtime/ctime timestamps
are not updated, violating POSIX security and semantics.

[Severity: Medium]
Does famfs_dax_write_iter() return the result of the write without calling
generic_write_sync() to perform the necessary cache flushes?
Without this, data written with synchronous flags (like O_SYNC or O_DSYNC)
will not be flushed from the CPU cache to persistent media, leading to
silent data loss upon power failure despite user requests for persistence.

>  	return rc;
>  }
>  
> +static int
> +famfs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct super_block *sb = inode->i_sb;
> +	struct famfs_fs_info *fsi = sb->s_fs_info;
> +	ssize_t rc;
> +
> +	if (fsi->deverror)
> +		return -ENODEV;
> +
> +	/*
> +	 * Gate shared-writable mappings on FAMFS_OPT_WRITE. This is best
> +	 * effort: clearing the bit blocks new writable mappings and write(),
> +	 * but does not revoke mappings that already exist.
> +	 */
> +	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE) &&
> +	    !famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))

[Severity: Medium]
Does this missing check for VM_MAYWRITE leave the VMA vulnerable to mprotect
upgrades?

If a user maps the file with PROT_READ when FAMFS_OPT_WRITE is disabled,
they could later bypass the access control and obtain a writable mapping
by calling mprotect() to PROT_WRITE.

> +		return -EPERM;
> +
> +	rc = famfs_file_invalid(inode);
> +	if (rc)
> +		return (int)rc;
> +
> +	file_accessed(file);
> +	vma->vm_ops = &famfs_file_vm_ops;
> +	vm_flags_set(vma, VM_HUGEPAGE);
> +	return 0;
> +}
> +
>  const struct file_operations famfs_file_operations = {
>  	.owner             = THIS_MODULE,
>  
> @@ -124,7 +223,7 @@ const struct file_operations famfs_file_operations = {
>  	.write_iter	   = famfs_dax_write_iter,
>  	.read_iter	   = famfs_dax_read_iter,
>  	.unlocked_ioctl    = NULL /*famfs_file_ioctl*/,
> -	.mmap		   = NULL /* famfs_file_mmap */,
> +	.mmap		   = famfs_file_mmap,

[Severity: Medium]
Does the file operations structure lack the required .fop_flags =
FOP_MMAP_SYNC?

Without this flag, the VFS will reject mmap() requests that include
MAP_SYNC | MAP_SHARED_VALIDATE with -EOPNOTSUPP, breaking the core DAX
use-case of userspace flushing without fsync.

>  
>  	/* Force PMD alignment for mmap */
>  	.get_unmapped_area = thp_get_unmapped_area,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=6

[PATCH V12 07/12] famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)

[John Groves]

From: John Groves <John@Groves.net>

Add the famfs file ioctl handler (FAMFSIOC_NOP, FAMFSIOC_MAP_CREATE) and
the KABI-44 self-describing fmap message: the wire ABI in famfs_ioctl.h
(famfs_ioc_fmap_header plus the simple and interleaved extent structs), the
in-core famfs_file_meta, and famfs_file_init_dax(), which copies the
message in, parses both the simple-extent and interleaved (striped) wire
forms into inode->i_private, and sets S_DAX.

Resolving those mappings to dax-device offsets (iomap_begin) is added in
the following commit; the read/write/fault paths keep their NULL iomap_ops
stub until then.

Also add famfs ioctls to ioctl-number.rst

Signed-off-by: John Groves <john@groves.net>
---
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 fs/famfs/famfs_file.c                         | 326 +++++++++++++++++-
 fs/famfs/famfs_inode.c                        |   1 +
 fs/famfs/famfs_internal.h                     |  46 +++
 include/uapi/linux/famfs_ioctl.h              |  91 +++++
 5 files changed, 462 insertions(+), 3 deletions(-)
 create mode 100644 include/uapi/linux/famfs_ioctl.h

diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 3f0ef1e27eb0..5e244dec1b98 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -299,6 +299,7 @@ Code  Seq#    Include File                                             Comments
 'u'   00-2F  linux/ublk_cmd.h                                          conflict!
 'u'   20-3F  linux/uvcvideo.h                                          USB video class host driver
 'u'   40-4f  linux/udmabuf.h                                           userspace dma-buf misc device
+'u'   50-5F  linux/famfs_ioctl.h                                       famfs shared memory file system
 'v'   00-1F  linux/ext2_fs.h                                           conflict!
 'v'   00-1F  linux/fs.h                                                conflict!
 'v'   00-0F  linux/sonypi.h                                            conflict!
diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index 678f2035fd5f..d710c8a0c923 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -13,9 +13,313 @@
 #include <linux/mm.h>
 #include <linux/dax.h>
 #include <linux/iomap.h>
+#include <linux/capability.h>
 
+#include <linux/famfs_ioctl.h>
 #include "famfs_internal.h"
 
+/* Expose famfs kernel abi version as a read-only module parameter */
+static int famfs_kabi_version = FAMFS_KABI_VERSION;
+module_param(famfs_kabi_version, int, 0444);
+MODULE_PARM_DESC(famfs_kabi_version, "famfs kernel abi version");
+
+void
+famfs_meta_free(struct famfs_file_meta *map)
+{
+	if (map) {
+		switch (map->fm_extent_type) {
+		case FAMFS_IOC_EXT_SIMPLE:
+			kfree(map->se);
+			break;
+		case FAMFS_IOC_EXT_INTERLEAVE:
+			if (map->ie) {
+				u32 i;
+
+				for (i = 0; i < map->fm_niext; i++)
+					kfree(map->ie[i].ie_strips);
+			}
+			kfree(map->ie);
+			break;
+		default:
+			break;
+		}
+	}
+	kfree(map);
+}
+
+/**
+ * famfs_file_init_dax() - FAMFSIOC_MAP_CREATE ioctl handler
+ * @file: the un-initialized file
+ * @arg:  user pointer to a self-describing fmap message
+ *
+ * The map-create ioctl carries the fmap as a self-describing message: a
+ * struct famfs_ioc_fmap_header followed by an extent list. The message is
+ * copied in, parsed into a famfs_file_meta, and published on inode->i_private.
+ * Both the simple-extent and the interleaved (striped) wire forms are handled.
+ * The wire layout byte-matches the fmap carried in a fuse famfs GET_FMAP reply.
+ */
+static int
+famfs_check_ext_alignment(struct famfs_meta_simple_ext *se)
+{
+	int errs = 0;
+
+	if (!IS_ALIGNED(se->ext_offset, PMD_SIZE))
+		errs++;
+	if (!IS_ALIGNED(se->ext_len, PMD_SIZE))
+		errs++;
+
+	return errs;
+}
+
+static int
+famfs_file_init_dax(struct file *file, void __user *arg)
+{
+	struct famfs_ioc_fmap_header fmh;
+	struct famfs_file_meta *meta = NULL;
+	struct famfs_fs_info *fsi;
+	struct super_block *sb;
+	struct inode *inode;
+	void *fmap_buf = NULL;
+	size_t extent_total = 0;
+	size_t next_offset;
+	int errs = 0;
+	int rc;
+	u32 i, j;
+
+	inode = file_inode(file);
+	if (!inode)
+		return -EBADF;
+	if (inode->i_private)
+		return -EEXIST;
+
+	sb  = inode->i_sb;
+	fsi = sb->s_fs_info;
+	if (fsi->deverror)
+		return -ENODEV;
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_MAP_CREATE))
+		return -EPERM;
+
+	if (copy_from_user(&fmh, arg, sizeof(fmh)))
+		return -EFAULT;
+
+	if (fmh.fmap_version != FAMFS_FMAP_VERSION)
+		return -EINVAL;
+	if (fmh.fmap_size < sizeof(fmh))
+		return -EINVAL;
+	if (fmh.fmap_size > FAMFS_FMAP_MSG_MAX)
+		return -EFBIG;
+	if (fmh.nextents < 1)
+		return -EINVAL;
+
+	fmap_buf = kvmalloc(fmh.fmap_size, GFP_KERNEL);
+	if (!fmap_buf)
+		return -ENOMEM;
+
+	if (copy_from_user(fmap_buf, arg, fmh.fmap_size)) {
+		rc = -EFAULT;
+		goto out;
+	}
+	next_offset = sizeof(fmh);	/* start of the extent list */
+
+	meta = kzalloc_obj(*meta, GFP_KERNEL);
+	if (!meta) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	meta->error = false;
+	meta->file_type = fmh.file_type;
+	meta->file_size = fmh.file_size;
+	meta->fm_extent_type = fmh.ext_type;
+
+	switch (fmh.ext_type) {
+	case FAMFS_IOC_EXT_SIMPLE: {
+		struct famfs_ioc_simple_ext *se_in = fmap_buf + next_offset;
+
+		next_offset += (size_t)fmh.nextents * sizeof(*se_in);
+		if (next_offset > fmh.fmap_size) {
+			rc = -EINVAL;
+			goto out;
+		}
+
+		meta->fm_nextents = fmh.nextents;
+		meta->se = kcalloc(meta->fm_nextents, sizeof(*meta->se),
+				   GFP_KERNEL);
+		if (!meta->se) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		for (i = 0; i < fmh.nextents; i++) {
+			meta->se[i].dev_index  = se_in[i].se_devindex;
+			meta->se[i].ext_offset = se_in[i].se_offset;
+			meta->se[i].ext_len    = se_in[i].se_len;
+
+			if (meta->se[i].dev_index >= FAMFS_MAX_DAXDEVS) {
+				rc = -EINVAL;
+				goto out;
+			}
+			meta->dev_bitmap |= BIT_ULL(meta->se[i].dev_index);
+			errs += famfs_check_ext_alignment(&meta->se[i]);
+			extent_total += meta->se[i].ext_len;
+		}
+		break;
+	}
+
+	case FAMFS_IOC_EXT_INTERLEAVE: {
+		s64 size_remainder = meta->file_size;
+		u32 niext = fmh.nextents;
+
+		meta->fm_niext = niext;
+		meta->ie = kcalloc(niext, sizeof(*meta->ie), GFP_KERNEL);
+		if (!meta->ie) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		/* Outer loop is over the separate interleaved extents */
+		for (i = 0; i < niext; i++) {
+			struct famfs_ioc_iext *ie_in = fmap_buf + next_offset;
+			struct famfs_ioc_simple_ext *sie_in;
+			u64 nstrips;
+
+			next_offset += sizeof(*ie_in);
+			if (next_offset > fmh.fmap_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			if (ie_in->ie_chunk_size == 0 ||
+			    !IS_ALIGNED(ie_in->ie_chunk_size, PMD_SIZE)) {
+				rc = -EINVAL;
+				goto out;
+			}
+			if (ie_in->ie_nbytes == 0) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			nstrips = ie_in->ie_nstrips;
+			if (nstrips < 1) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			meta->ie[i].fie_chunk_size = ie_in->ie_chunk_size;
+			meta->ie[i].fie_nstrips    = ie_in->ie_nstrips;
+			meta->ie[i].fie_nbytes     = ie_in->ie_nbytes;
+
+			/* The strip extents follow the interleaved-ext header */
+			sie_in = fmap_buf + next_offset;
+			next_offset += nstrips * sizeof(*sie_in);
+			if (next_offset > fmh.fmap_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			meta->ie[i].ie_strips =
+				kcalloc(nstrips, sizeof(meta->ie[i].ie_strips[0]),
+					GFP_KERNEL);
+			if (!meta->ie[i].ie_strips) {
+				rc = -ENOMEM;
+				goto out;
+			}
+
+			/* Inner loop is over the strips */
+			for (j = 0; j < nstrips; j++) {
+				struct famfs_meta_simple_ext *so =
+					&meta->ie[i].ie_strips[j];
+
+				so->dev_index  = sie_in[j].se_devindex;
+				so->ext_offset = sie_in[j].se_offset;
+				so->ext_len    = sie_in[j].se_len;
+
+				if (so->dev_index >= FAMFS_MAX_DAXDEVS) {
+					rc = -EINVAL;
+					goto out;
+				}
+				meta->dev_bitmap |= BIT_ULL(so->dev_index);
+				errs += famfs_check_ext_alignment(so);
+				extent_total += so->ext_len;
+				size_remainder -= so->ext_len;
+			}
+		}
+
+		if (size_remainder > 0) {
+			/* Strips do not cover the whole file */
+			rc = -EINVAL;
+			goto out;
+		}
+		break;
+	}
+
+	default:
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (errs > 0) {
+		rc = -EINVAL;
+		goto out;
+	}
+	if (extent_total < meta->file_size) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	/* Publish the famfs metadata on inode->i_private */
+	inode_lock(inode);
+	if (inode->i_private) {
+		rc = -EEXIST; /* file already has famfs metadata */
+	} else {
+		inode->i_private = meta;
+		i_size_write(inode, meta->file_size);
+		inode->i_flags |= S_DAX;
+		meta = NULL; /* owned by the inode now */
+		rc = 0;
+	}
+	inode_unlock(inode);
+
+out:
+	kvfree(fmap_buf);
+	if (meta)
+		famfs_meta_free(meta);
+	return rc;
+}
+
+/**
+ * famfs_file_ioctl() - Top-level famfs file ioctl handler
+ * @file: the file
+ * @cmd:  ioctl opcode
+ * @arg:  ioctl opcode argument (if any)
+ */
+static long
+famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct inode *inode = file_inode(file);
+	struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
+	long rc;
+
+	if (fsi->deverror && (cmd != FAMFSIOC_NOP))
+		return -ENODEV;
+
+	switch (cmd) {
+	case FAMFSIOC_NOP:
+		rc = 0;
+		break;
+
+	case FAMFSIOC_MAP_CREATE:
+		rc = famfs_file_init_dax(file, (void __user *)arg);
+		break;
+
+	default:
+		rc = -ENOTTY;
+		break;
+	}
+
+	return rc;
+}
+
 /*********************************************************************
  * vm_operations
  */
@@ -93,9 +397,25 @@ const struct vm_operations_struct famfs_file_vm_ops = {
 static ssize_t
 famfs_file_invalid(struct inode *inode)
 {
+	struct famfs_file_meta *meta = inode->i_private;
+	size_t i_size = i_size_read(inode);
+
+	if (!meta) {
+		pr_debug("%s: un-initialized famfs file\n", __func__);
+		return -EIO;
+	}
+	if (meta->error) {
+		pr_debug("%s: previously detected metadata errors\n", __func__);
+		return -EIO;
+	}
+	if (i_size != meta->file_size) {
+		pr_warn("%s: i_size overwritten from %ld to %ld\n",
+		       __func__, meta->file_size, i_size);
+		meta->error = true;
+		return -ENXIO;
+	}
 	if (!IS_DAX(inode)) {
-		pr_debug("%s: inode %llx IS_DAX is false\n",
-			 __func__, (u64)inode);
+		pr_debug("%s: inode %llx IS_DAX is false\n", __func__, (u64)inode);
 		return -ENXIO;
 	}
 	return 0;
@@ -222,7 +542,7 @@ const struct file_operations famfs_file_operations = {
 	/* Custom famfs operations */
 	.write_iter	   = famfs_dax_write_iter,
 	.read_iter	   = famfs_dax_read_iter,
-	.unlocked_ioctl    = NULL /*famfs_file_ioctl*/,
+	.unlocked_ioctl    = famfs_file_ioctl,
 	.mmap		   = famfs_file_mmap,
 
 	/* Force PMD alignment for mmap */
diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index 910a143dad30..a6c3b4574e69 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -300,6 +300,7 @@ static int famfs_show_options(struct seq_file *m, struct dentry *root)
 
 static void famfs_evict_inode(struct inode *inode)
 {
+	famfs_meta_free((struct famfs_file_meta *)inode->i_private);
 	inode->i_private = NULL;
 	dax_break_layout_final(inode);
 	truncate_inode_pages_final(&inode->i_data);
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index 26f5abda96dc..b5f9c8d0349f 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -15,8 +15,52 @@
 #include <linux/bits.h>
 #include <linux/build_bug.h>
 
+#include <linux/famfs_ioctl.h>
+
 extern const struct file_operations famfs_file_operations;
 
+/*
+ * Internal sanity bound on a FAMFSIOC_MAP_CREATE fmap message. The ABI does
+ * not advertise a maximum (the message is self-describing); this only guards
+ * the copy-in against an unreasonable allocation. Oversize is rejected with
+ * -EFBIG.
+ */
+#define FAMFS_FMAP_MSG_MAX (4 * 1024 * 1024)
+
+struct famfs_meta_simple_ext {
+	u64 dev_index;
+	u64 ext_offset;
+	u64 ext_len;
+};
+
+struct famfs_meta_interleaved_ext {
+	u64 fie_nstrips;
+	u64 fie_chunk_size;
+	u64 fie_nbytes;
+	struct famfs_meta_simple_ext *ie_strips;
+};
+
+/*
+ * Each famfs dax file has this hanging from its inode->i_private.
+ */
+struct famfs_file_meta {
+	bool                   error;
+	enum famfs_file_type   file_type;
+	size_t                 file_size;
+	enum famfs_ioc_ext_type fm_extent_type;
+	u64                    dev_bitmap; /* referenced daxdev indices */
+	union { /* This will make code a bit more readable */
+		struct {
+			size_t         fm_nextents;
+			struct famfs_meta_simple_ext  *se;
+		};
+		struct {
+			size_t         fm_niext;
+			struct famfs_meta_interleaved_ext *ie;
+		};
+	};
+};
+
 struct famfs_mount_opts {
 	umode_t mode;
 };
@@ -83,4 +127,6 @@ int famfs_devlist_alloc(struct famfs_fs_info *fsi);
 int famfs_install_daxdev(struct famfs_fs_info *fsi, struct super_block *sb,
 			 u64 index, dev_t devno, const char *name);
 
+void famfs_meta_free(struct famfs_file_meta *map);
+
 #endif /* FAMFS_INTERNAL_H */
diff --git a/include/uapi/linux/famfs_ioctl.h b/include/uapi/linux/famfs_ioctl.h
new file mode 100644
index 000000000000..b4eb373c1ade
--- /dev/null
+++ b/include/uapi/linux/famfs_ioctl.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * famfs - dax file system for shared fabric-attached memory
+ *
+ * Copyright 2023-2024 Micron Technology, Inc.
+ *
+ * This file system, originally based on ramfs the dax support from xfs,
+ * is intended to allow multiple host systems to mount a common file system
+ * view of dax files that map to shared memory.
+ */
+#ifndef FAMFS_IOCTL_H
+#define FAMFS_IOCTL_H
+
+#include <linux/ioctl.h>
+#include <linux/uuid.h>
+
+#define FAMFS_KABI_VERSION 44
+
+enum famfs_file_type {
+	FAMFS_REG,
+	FAMFS_SUPERBLOCK,
+	FAMFS_LOG,
+};
+
+/*
+ * Extent type in a famfs fmap message, and of the in-core map
+ * (famfs_file_meta.fm_extent_type).
+ */
+enum famfs_ioc_ext_type {
+	FAMFS_IOC_EXT_SIMPLE,
+	FAMFS_IOC_EXT_INTERLEAVE,
+};
+
+/*
+ * The FAMFSIOC_MAP_CREATE payload is a self-describing fmap message: a
+ * struct famfs_ioc_fmap_header immediately followed by @nextents extent
+ * records. @fmap_size gives the total message length, so a reader is
+ * self-delimiting.
+ *
+ * For ext_type == FAMFS_IOC_EXT_SIMPLE the records are an array of
+ * @nextents famfs_ioc_simple_ext. For ext_type == FAMFS_IOC_EXT_INTERLEAVE
+ * each of the @nextents records is a famfs_ioc_iext header immediately
+ * followed by ie_nstrips famfs_ioc_simple_ext strip extents.
+ *
+ * This wire layout is byte-identical to the fmap carried in a fuse famfs
+ * GET_FMAP reply, so the same userspace serializer emits both.
+ *
+ * The message is self-describing (@fmap_size bounds it), so neither the extent
+ * and strip counts nor the total size are capped by this ABI. The kernel
+ * applies an internal sanity limit to the copy-in and returns -EFBIG for a
+ * message larger than it will accept.
+ */
+#define FAMFS_FMAP_VERSION 1
+
+struct famfs_ioc_simple_ext {
+	__u32 se_devindex;
+	__u32 reserved;
+	__u64 se_offset;
+	__u64 se_len;
+};
+
+struct famfs_ioc_iext {		/* interleaved (striped) extent */
+	__u32 ie_nstrips;
+	__u32 ie_chunk_size;
+	__u64 ie_nbytes;	/* total bytes mapped by this interleaved extent */
+	__u64 reserved;
+};
+
+struct famfs_ioc_fmap_header {
+	__u8  file_type;	/* enum famfs_file_type */
+	__u8  reserved;
+	__u16 fmap_version;	/* FAMFS_FMAP_VERSION */
+	__u32 ext_type;		/* enum famfs_ioc_ext_type */
+	__u32 nextents;
+	__u32 fmap_size;	/* total message bytes, including this header */
+	__u64 file_size;
+	__u64 reserved1;
+};
+
+#define FAMFSIOC_MAGIC 'u'
+
+/* famfs file ioctl opcodes */
+#define FAMFSIOC_NOP           _IO(FAMFSIOC_MAGIC,   0x50)
+
+/*
+ * MAP_CREATE carries the self-describing fmap message - struct
+ * famfs_ioc_fmap_header followed by the extent list (see above).
+ */
+#define FAMFSIOC_MAP_CREATE    _IOW(FAMFSIOC_MAGIC,  0x51, struct famfs_ioc_fmap_header)
+
+#endif /* FAMFS_IOCTL_H */
-- 
2.53.0

Re: [PATCH V12 07/12] famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [Critical] The FAMFSIOC_MAP_CREATE ioctl lacks a privilege check, allowing unprivileged users to map arbitrary physical DAX memory.
- [High] 64-bit file size and extent lengths are silently truncated to size_t on 32-bit architectures.
- [High] Missing overflow checks on extent lengths and physical offsets.
- [High] Race condition in state publication leads to permanent file corruption (Denial of Service).
- [High] Persistent memory allocations for file metadata lack __GFP_ACCOUNT, bypassing memcg limits.
- [Low] Format string type mismatch for size_t arguments causes compiler warnings.

Pre-existing issues:
- [High] NULL pointer dereference in famfs_meta_to_dax_offset_interleaved.
--

--- Patch [7]: [PATCH V12 07/12] famfs: MAP_CREATE ioctl and fmap ingest (ABI 44) ---
commit 7c28fdf850baa1b17b165378da6b0515248fad98
Author: John Groves <John@Groves.net>

famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)

This commit adds the famfs file ioctl handler, including the
FAMFSIOC_MAP_CREATE ioctl to ingest user-provided mapping extent messages.
It parses both simple and interleaved wire formats into a famfs_file_meta
structure that is published on the inode.

> diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
> --- a/fs/famfs/famfs_internal.h
> +++ b/fs/famfs/famfs_internal.h
> @@ -15,8 +15,52 @@
>  #include <linux/bits.h>
>  #include <linux/build_bug.h>
>  
> +#include <linux/famfs_ioctl.h>
> +
>  extern const struct file_operations famfs_file_operations;
[ ... ]
> +struct famfs_file_meta {
> +	bool                   error;
> +	enum famfs_file_type   file_type;
> +	size_t                 file_size;

[Severity: High]
Will this type choice truncate the 64-bit file size to 32 bits on 32-bit
architectures? If fmh.file_size exceeds 4GB, the upper 32 bits will be
lost, which could silently corrupt the file size and allow validation
checks to be bypassed.

> +	enum famfs_ioc_ext_type fm_extent_type;
> +	u64                    dev_bitmap; /* referenced daxdev indices */
> +	union { /* This will make code a bit more readable */
> +		struct {
> +			size_t         fm_nextents;
> +			struct famfs_meta_simple_ext  *se;
> +		};
> +		struct {
> +			size_t         fm_niext;
> +			struct famfs_meta_interleaved_ext *ie;
> +		};
> +	};
> +};

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
[ ... ]
> @@ -60,6 +81,250 @@
> +static int
> +famfs_file_init_dax(struct file *file, void __user *arg)
> +{
[ ... ]
> +	fmap_buf = kvmalloc(fmh.fmap_size, GFP_KERNEL);

[Severity: High]
Should these metadata allocations use the __GFP_ACCOUNT flag?
This allocation scales with user input. Without __GFP_ACCOUNT, an
unprivileged user mapping up to 4MB of metadata per file could bypass
memory cgroup limits and potentially trigger a host-wide out-of-memory
condition.

> +	if (!fmap_buf)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(fmap_buf, arg, fmh.fmap_size)) {
> +		rc = -EFAULT;
> +		goto out;
> +	}
> +	next_offset = sizeof(fmh);	/* start of the extent list */
> +
> +	meta = kzalloc_obj(*meta, GFP_KERNEL);
> +	if (!meta) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	meta->error = false;
> +	meta->file_type = fmh.file_type;
> +	meta->file_size = fmh.file_size;
> +	meta->fm_extent_type = fmh.ext_type;
> +
> +	switch (fmh.ext_type) {
> +	case FAMFS_IOC_EXT_SIMPLE: {
> +		struct famfs_ioc_simple_ext *se_in = fmap_buf + next_offset;
[ ... ]
> +		for (i = 0; i < fmh.nextents; i++) {
> +			meta->se[i].dev_index  = se_in[i].se_devindex;
> +			meta->se[i].ext_offset = se_in[i].se_offset;
> +			meta->se[i].ext_len    = se_in[i].se_len;
> +
> +			if (meta->se[i].dev_index >= FAMFS_MAX_DAXDEVS) {
> +				rc = -EINVAL;
> +				goto out;
> +			}
> +			meta->dev_bitmap |= BIT_ULL(meta->se[i].dev_index);
> +			errs += famfs_check_ext_alignment(&meta->se[i]);
> +			extent_total += meta->se[i].ext_len;

[Severity: High]
Is there a potential integer overflow when accumulating extent_total here?
If the sum of ext_len wraps around, it might pass the extent_total check below
even if the extents do not actually cover the file size. Could ext_offset +
ext_len also overflow U64_MAX and cause out-of-bounds physical memory
accesses when constructing the iomap?

> +		}
> +		break;
> +	}
[ ... ]
> +	/* Publish the famfs metadata on inode->i_private */
> +	inode_lock(inode);
> +	if (inode->i_private) {
> +		rc = -EEXIST; /* file already has famfs metadata */
> +	} else {
> +		inode->i_private = meta;
> +		i_size_write(inode, meta->file_size);

[Severity: High]
Could a concurrent reader observe the published metadata but a stale i_size?
Because inode->i_private is assigned before i_size_write(), a lockless
reader could call famfs_file_invalid() in the middle of this window. It
would see a size mismatch and incorrectly set meta->error = true, which
permanently marks the file as corrupted.

> +		inode->i_flags |= S_DAX;
> +		meta = NULL; /* owned by the inode now */
> +		rc = 0;
> +	}
> +	inode_unlock(inode);
> +
> +out:
> +	kvfree(fmap_buf);
> +	if (meta)
> +		famfs_meta_free(meta);
> +	return rc;
> +}
> +
> +/**
> + * famfs_file_ioctl() - Top-level famfs file ioctl handler
> + * @file: the file
> + * @cmd:  ioctl opcode
> + * @arg:  ioctl opcode argument (if any)
> + */
> +static long
> +famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
> +	long rc;
> +
> +	if (fsi->deverror && (cmd != FAMFSIOC_NOP))
> +		return -ENODEV;
> +
> +	switch (cmd) {
> +	case FAMFSIOC_NOP:
> +		rc = 0;
> +		break;
> +
> +	case FAMFSIOC_MAP_CREATE:
> +		rc = famfs_file_init_dax(file, (void __user *)arg);

[Severity: Critical]
Does this ioctl command require a privilege check like capable(CAP_SYS_ADMIN)?
Without it, it appears any unprivileged user who can open the file could map
arbitrary physical DAX memory directly into their processes.

> +		break;
> +
> +	default:
> +		rc = -ENOTTY;
> +		break;
> +	}
> +
> +	return rc;
> +}
[ ... ]
> @@ -93,9 +397,25 @@ const struct vm_operations_struct famfs_file_vm_ops = {
>  static ssize_t
>  famfs_file_invalid(struct inode *inode)
>  {
> +	struct famfs_file_meta *meta = inode->i_private;
> +	size_t i_size = i_size_read(inode);
> +
> +	if (!meta) {
> +		pr_debug("%s: un-initialized famfs file\n", __func__);
> +		return -EIO;
> +	}
> +	if (meta->error) {
> +		pr_debug("%s: previously detected metadata errors\n", __func__);
> +		return -EIO;
> +	}
> +	if (i_size != meta->file_size) {
> +		pr_warn("%s: i_size overwritten from %ld to %ld\n",
> +		       __func__, meta->file_size, i_size);

[Severity: Low]
Should this format string use %zu instead of %ld for size_t variables?
This can cause compiler warnings and misformatted logs on 32-bit platforms.

> +		meta->error = true;
> +		return -ENXIO;
> +	}

[ ... ]

[Severity: High]
This is a pre-existing issue, but can famfs_meta_to_dax_offset_interleaved()
dereference a NULL pointer if called on an uninitialized file?

fs/famfs/famfs_file.c:famfs_meta_to_dax_offset_interleaved() {
    ...
    struct famfs_file_meta *meta = inode->i_private;
    ...
    if (meta->fm_extent_type != FAMFS_IOC_EXT_INTERLEAVE) {
        pr_debug("%s: bad extent type\n", __func__);
        goto err_out;
    }

    if (fsi->deverror || famfs_file_invalid(inode))
        goto err_out;
    ...
}

The meta pointer is dereferenced to check fm_extent_type before the call to
famfs_file_invalid(), which is where the check for a NULL meta pointer
actually happens. If the file is uninitialized (inode->i_private is NULL),
this will cause a panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=7

[PATCH V12 08/12] famfs: iomap_begin and file-to-dax offset resolution

[John Groves]

From: John Groves <John@Groves.net>

Add the iomap resolver that maps a file offset to a (daxdev, offset) pair:
famfs_meta_to_dax_offset() for simple extent lists and
famfs_meta_to_dax_offset_interleaved() for striped files, backed by the
per-daxdev health check (famfs_dax_err) and table lookup
(famfs_daxdev_for_index), plus famfs_iomap_begin() and famfs_iomap_ops.

Wire it into the read, write and fault paths by replacing their
NULL /*&famfs_iomap_ops*/ stub with &famfs_iomap_ops, so dax_iomap_rw() and
dax_iomap_fault() now resolve through famfs.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_file.c | 298 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 295 insertions(+), 3 deletions(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index d710c8a0c923..e7f271ce6d03 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -320,6 +320,298 @@ famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return rc;
 }
 
+/*********************************************************************
+ * iomap_operations
+ *
+ * This stuff uses the iomap (dax-related) helpers to resolve file offsets to
+ * offsets within a dax device.
+ */
+
+static ssize_t famfs_file_invalid(struct inode *inode);
+
+/* Check the health of a daxdev table slot */
+static int famfs_dax_err(struct famfs_daxdev *dd)
+{
+	if (!dd->valid) {
+		pr_debug("%s: daxdev=%s invalid\n", __func__, dd->name);
+		return -EIO;
+	}
+	if (dd->dax_err) {
+		pr_debug("%s: daxdev=%s dax_err\n", __func__, dd->name);
+		return -EIO;
+	}
+	if (dd->error) {
+		pr_debug("%s: daxdev=%s memory error\n", __func__, dd->name);
+		return -EHWPOISON;
+	}
+	return 0;
+}
+
+/*
+ * famfs_daxdev_from_index() - resolve an extent's dev_index to a health-checked
+ * dax_device from the table. On success returns the dax_device and sets
+ * *errp = 0; on failure returns NULL and sets *errp (< 0).
+ */
+static struct dax_device *
+famfs_daxdev_from_index(struct famfs_fs_info *fsi, u64 dev_index, int *errp)
+{
+	struct famfs_dax_devlist *devlist = fsi->dax_devlist;
+	struct famfs_daxdev *dd;
+	int rc;
+
+	if (!devlist || dev_index >= devlist->nslots) {
+		pr_debug("%s: dev_index %llu out of range\n",
+			__func__, dev_index);
+		*errp = -EIO;
+		return NULL;
+	}
+	dd = &devlist->devlist[dev_index];
+	rc = famfs_dax_err(dd);
+	if (rc) {
+		*errp = rc;
+		return NULL;
+	}
+	*errp = 0;
+	return dd->devp;
+}
+
+static int
+famfs_meta_to_dax_offset_interleaved(struct inode *inode, struct iomap *iomap,
+			 loff_t file_offset, off_t len, unsigned int flags)
+{
+	struct famfs_fs_info  *fsi = inode->i_sb->s_fs_info;
+	struct famfs_file_meta *meta = inode->i_private;
+	loff_t local_offset = file_offset;
+	int rc;
+	int i;
+
+	/* This function is only for extent_type FAMFS_IOC_EXT_INTERLEAVE */
+	if (meta->fm_extent_type != FAMFS_IOC_EXT_INTERLEAVE) {
+		pr_debug("%s: bad extent type\n", __func__);
+		goto err_out;
+	}
+
+	if (fsi->deverror || famfs_file_invalid(inode))
+		goto err_out;
+
+	iomap->offset = file_offset;
+
+	for (i = 0; i < meta->fm_niext; i++) {
+		struct famfs_meta_interleaved_ext *fei = &meta->ie[i];
+		u64 chunk_size = fei->fie_chunk_size;
+		u64 nstrips = fei->fie_nstrips;
+		u64 ext_size = fei->fie_nbytes;
+
+		ext_size = min_t(u64, ext_size, meta->file_size);
+
+		if (ext_size == 0)
+			goto err_out;
+
+		/* Is the data is in this striped extent? */
+		if (local_offset < ext_size) {
+			u64 chunk_num       = local_offset / chunk_size;
+			u64 chunk_offset    = local_offset % chunk_size;
+			u64 stripe_num      = chunk_num / nstrips;
+			u64 strip_num       = chunk_num % nstrips;
+			u64 chunk_remainder = chunk_size - chunk_offset;
+			u64 strip_offset    = chunk_offset + (stripe_num * chunk_size);
+			struct famfs_meta_simple_ext *strip = &fei->ie_strips[strip_num];
+			struct dax_device *daxdev;
+
+			/*
+			 * MAP_CREATE only checks that the strips' combined
+			 * length covers the file, not that each strip is large
+			 * enough for the chunks striped onto it. Guard against a
+			 * malformed fmap with an undersized strip so we never
+			 * resolve to a dax offset past the strip's extent.
+			 */
+			if (strip_offset >= strip->ext_len)
+				goto err_out;
+
+			daxdev = famfs_daxdev_from_index(fsi, strip->dev_index, &rc);
+			if (!daxdev) {
+				meta->error = true;
+				return rc;
+			}
+
+			iomap->addr    = strip->ext_offset + strip_offset;
+			iomap->offset  = file_offset;
+			iomap->length  = min_t(loff_t, len, chunk_remainder);
+			iomap->length  = min_t(loff_t, iomap->length,
+					       strip->ext_len - strip_offset);
+			iomap->dax_dev = daxdev;
+			iomap->type    = IOMAP_MAPPED;
+			iomap->flags   = flags;
+
+			return 0;
+		}
+		local_offset -= ext_size; /* offset is beyond this striped extent */
+	}
+
+ err_out:
+	/*
+	 * We fell out the end of the extent list (access past EOF) or the file
+	 * is invalid. Return -EIO: iomap requires a non-zero-length mapping on
+	 * success (iomap_iter_done() warns on length == 0), so signal the error
+	 * rather than returning a zero-length IOMAP_MAPPED.
+	 */
+	pr_debug("%s: could not resolve file_offset %lld (past EOF?)\n",
+		 __func__, (long long)file_offset);
+
+	iomap->addr    = 0; /* there is no valid dax device offset */
+	iomap->offset  = file_offset; /* file offset */
+	iomap->length  = 0;
+	iomap->dax_dev = famfs_daxdev_from_index(fsi, 0, &rc);
+	iomap->type    = IOMAP_MAPPED;
+	iomap->flags   = flags;
+
+	return -EIO;
+}
+
+/**
+ * famfs_meta_to_dax_offset() - Resolve (file, offset, len) to (daxdev, offset, len)
+ *
+ * This function is called by famfs_iomap_begin() to resolve an offset in a
+ * file to an offset in a dax device. This is upcalled from dax from calls to
+ * both  * dax_iomap_fault() and dax_iomap_rw(). Dax finishes the job resolving
+ * a fault to a specific physical page (the fault case) or doing a memcpy
+ * variant (the rw case)
+ *
+ * Pages can be PTE (4k), PMD (2MiB) or (theoretically) PuD (1GiB)
+ * (these sizes are for X86; may vary on other cpu architectures
+ *
+ * @inode:  The file where the fault occurred
+ * @iomap:       To be filled in to indicate where to find the right memory,
+ *               relative  to a dax device.
+ * @file_offset: Within the file where the fault occurred (will be page boundary)
+ * @len:         The length of the faulted mapping (will be a page multiple)
+ *               (will be trimmed in *iomap if it's disjoint in the extent list)
+ * @flags:
+ *
+ * Return values: 0. (info is returned in a modified @iomap struct)
+ */
+static int
+famfs_meta_to_dax_offset(struct inode *inode, struct iomap *iomap,
+			 loff_t file_offset, off_t len, unsigned int flags)
+{
+	struct famfs_fs_info  *fsi = inode->i_sb->s_fs_info;
+	struct famfs_file_meta *meta = inode->i_private;
+	loff_t local_offset = file_offset;
+	int rc;
+	int i;
+
+	if (fsi->deverror || famfs_file_invalid(inode))
+		goto err_out;
+
+	if (meta->fm_extent_type == FAMFS_IOC_EXT_INTERLEAVE)
+		return famfs_meta_to_dax_offset_interleaved(inode,
+					iomap, file_offset, len, flags);
+
+	if (meta->fm_extent_type != FAMFS_IOC_EXT_SIMPLE)
+		goto err_out;
+
+	iomap->offset = file_offset;
+
+	for (i = 0; i < meta->fm_nextents; i++) {
+		loff_t dax_ext_offset = meta->se[i].ext_offset;
+		loff_t dax_ext_len    = meta->se[i].ext_len;
+
+		if ((dax_ext_offset == 0) &&
+		    (meta->file_type != FAMFS_SUPERBLOCK))
+			pr_warn("%s: zero offset on non-superblock file!!\n",
+				__func__);
+
+		/* local_offset is the offset minus the size of extents skipped
+		 * so far; If local_offset < dax_ext_len, the data of interest
+		 * starts in this extent
+		 */
+		if (local_offset < dax_ext_len) {
+			loff_t ext_len_remainder = dax_ext_len - local_offset;
+			struct dax_device *daxdev;
+
+			daxdev = famfs_daxdev_from_index(fsi,
+						meta->se[i].dev_index, &rc);
+			if (!daxdev) {
+				meta->error = true;
+				return rc;
+			}
+
+			/*
+			 * OK, we found the file metadata extent where this
+			 * data begins
+			 * @local_offset      - The offset within the current
+			 *                      extent
+			 * @ext_len_remainder - Remaining length of ext after
+			 *                      skipping local_offset
+			 * Outputs:
+			 * iomap->addr:   the offset within the dax device where
+			 *                the  data starts
+			 * iomap->offset: the file offset
+			 * iomap->length: the valid length resolved here
+			 */
+			iomap->addr    = dax_ext_offset + local_offset;
+			iomap->offset  = file_offset;
+			iomap->length  = min_t(loff_t, len, ext_len_remainder);
+			iomap->dax_dev = daxdev;
+			iomap->type    = IOMAP_MAPPED;
+			iomap->flags   = flags;
+
+			return 0;
+		}
+		local_offset -= dax_ext_len; /* Get ready for the next extent */
+	}
+
+ err_out:
+	/*
+	 * We fell out the end of the extent list (access past EOF) or the file
+	 * is in an invalid state. Return -EIO: iomap requires a non-zero-length
+	 * mapping on success (iomap_iter_done() warns on length == 0), so signal
+	 * the error rather than returning a zero-length IOMAP_MAPPED. dax turns
+	 * this into a short read/write or a SIGBUS.
+	 */
+	pr_debug("%s: could not resolve file_offset %lld (past EOF?)\n",
+		 __func__, (long long)file_offset);
+
+	iomap->addr    = 0; /* there is no valid dax device offset */
+	iomap->offset  = file_offset; /* file offset */
+	iomap->length  = 0;
+	iomap->dax_dev = famfs_daxdev_from_index(fsi, 0, &rc);
+	iomap->type    = IOMAP_MAPPED;
+	iomap->flags   = flags;
+
+	return -EIO;
+}
+
+/**
+ * famfs_iomap_begin() - Handler for iomap_begin upcall from dax
+ *
+ * This function is pretty simple because files are
+ * * never partially allocated
+ * * never have holes (never sparse)
+ * * never "allocate on write"
+ *
+ * @inode:  inode for the file being accessed
+ * @offset: offset within the file
+ * @length: Length being accessed at offset
+ * @flags:
+ * @iomap:  iomap struct to be filled in, resolving (offset, length) to
+ *          (daxdev, offset, len)
+ * @srcmap:
+ */
+static int
+famfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+		  unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
+{
+	return famfs_meta_to_dax_offset(inode, iomap, offset, length, flags);
+}
+
+/* Note: We never need a special set of write_iomap_ops because famfs never
+ * performs allocation on write.
+ */
+const struct iomap_ops famfs_iomap_ops = {
+	.iomap_begin		= famfs_iomap_begin,
+};
+
 /*********************************************************************
  * vm_operations
  */
@@ -346,7 +638,7 @@ __famfs_filemap_fault(struct vm_fault *vmf, unsigned int order,
 		file_update_time(vmf->vma->vm_file);
 	}
 
-	ret = dax_iomap_fault(vmf, order, &pfn, NULL, NULL /*&famfs_iomap_ops */);
+	ret = dax_iomap_fault(vmf, order, &pfn, NULL, &famfs_iomap_ops);
 	if (ret & VM_FAULT_NEEDDSYNC)
 		ret = dax_finish_sync_fault(vmf, order, pfn);
 
@@ -468,7 +760,7 @@ famfs_dax_read_iter(struct kiocb *iocb, struct iov_iter	*to)
 		return rc;
 	}
 
-	rc = dax_iomap_rw(iocb, to, NULL /*&famfs_iomap_ops */);
+	rc = dax_iomap_rw(iocb, to, &famfs_iomap_ops);
 	inode_unlock_shared(inode);
 
 	file_accessed(iocb->ki_filp);
@@ -501,7 +793,7 @@ famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		return rc;
 	}
 
-	rc = dax_iomap_rw(iocb, from, NULL /*&famfs_iomap_ops*/);
+	rc = dax_iomap_rw(iocb, from, &famfs_iomap_ops);
 	inode_unlock(inode);
 	return rc;
 }
-- 
2.53.0

Re: [PATCH V12 08/12] famfs: iomap_begin and file-to-dax offset resolution

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:

New issues:
- [Critical] Missing read memory barrier (`smp_rmb()`) in `famfs_daxdev_from_index` allows returning an uninitialized pointer, causing the DAX core to use an uninitialized iomap structure.
- [Critical] The iomap_begin handlers mistakenly assign the input operation `flags` to the output `iomap->flags`, unintentionally setting `IOMAP_F_NEW` during writes and causing data corruption.
- [High] The `len` parameter in `famfs_meta_to_dax_offset` and `famfs_meta_to_dax_offset_interleaved` is incorrectly typed as `off_t` instead of `loff_t`, causing integer truncation on 32-bit architectures.
- [High] In `famfs_meta_to_dax_offset_interleaved`, `iomap->length` is not bounded by the end of the interleaved extent, causing out-of-bounds physical mappings.
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race condition in `famfs_meta_to_dax_offset` allows a NULL pointer dereference when evaluating `inode->i_private`.
- [Low] 64-bit integer division and modulo operations in `famfs_meta_to_dax_offset_interleaved` cause unresolved symbol errors during linking on 32-bit architectures.

Pre-existing issues:
- [Critical] The `FAMFSIOC_MAP_CREATE` ioctl lacks privilege checks, allowing any user with write access to a file to map and access arbitrary physical memory on any registered DAX device.
- [High] `famfs_dax_write_iter` bypasses standard VFS write checks, failing to clear setuid/setgid bits and update file times upon modification.
--

--- Patch [8]: [PATCH V12 08/12] famfs: iomap_begin and file-to-dax offset resolution ---
commit b4e4f3f2df1ac1714b2f127455834eca42345a35
Author: John Groves <John@Groves.net>
Subject: famfs: iomap_begin and file-to-dax offset resolution

Add the iomap resolver that maps a file offset to a (daxdev, offset) pair:
famfs_meta_to_dax_offset() for simple extent lists and
famfs_meta_to_dax_offset_interleaved() for striped files, backed by the
per-daxdev health check (famfs_dax_err) and table lookup
(famfs_daxdev_for_index), plus famfs_iomap_begin() and famfs_iomap_ops.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index d710c8a0c923c..e7f271ce6d03d 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
> @@ -320,6 +320,298 @@ famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
[ ... ]
> +static struct dax_device *
> +famfs_daxdev_from_index(struct famfs_fs_info *fsi, u64 dev_index, int *errp)
> +{
> +	struct famfs_dax_devlist *devlist = fsi->dax_devlist;
> +	struct famfs_daxdev *dd;
> +	int rc;
> +
> +	if (!devlist || dev_index >= devlist->nslots) {
> +		pr_debug("%s: dev_index %llu out of range\n",
> +			__func__, dev_index);
> +		*errp = -EIO;
> +		return NULL;
> +	}
> +	dd = &devlist->devlist[dev_index];
> +	rc = famfs_dax_err(dd);
> +	if (rc) {
> +		*errp = rc;
> +		return NULL;
> +	}
> +	*errp = 0;
> +	return dd->devp;
> +}

[Severity: Critical]
Does this code need a read memory barrier?

In famfs_daxdev_from_index(), dd->valid is read via famfs_dax_err(), and
then dd->devp is returned without an smp_rmb(). If a concurrent page fault
occurs while a DAX device is being installed via famfs_install_daxdev(),
CPU reordering could cause this to see valid == 1 but return a NULL device
pointer.

[ ... ]
> +static int
> +famfs_meta_to_dax_offset_interleaved(struct inode *inode, struct iomap *iomap,
> +			 loff_t file_offset, off_t len, unsigned int flags)

[Severity: High]
Can this cause integer truncation on 32-bit architectures?

The len parameter is typed as off_t (which is 32-bit signed on 32-bit
architectures) instead of loff_t. A large I/O request could result in a
negative value, which is then sign-extended back to a 64-bit value in
min_t(loff_t, len, ...), resulting in a massive iomap->length and causing
out-of-bounds mappings. Should this match the iomap_begin signature with
loff_t?

[ ... ]
> +		/* Is the data is in this striped extent? */
> +		if (local_offset < ext_size) {
> +			u64 chunk_num       = local_offset / chunk_size;
> +			u64 chunk_offset    = local_offset % chunk_size;
> +			u64 stripe_num      = chunk_num / nstrips;
> +			u64 strip_num       = chunk_num % nstrips;

[Severity: Low]
Could these 64-bit integer division and modulo operations cause unresolved
symbol errors during linking on 32-bit architectures?

The code uses standard / and % operators on 64-bit integers (local_offset,
chunk_size, chunk_num), which emits calls to __divdi3 and __umoddi3. Should
this use the kernel division helpers like div64_u64() or do_div()?

[ ... ]
> +			iomap->addr    = strip->ext_offset + strip_offset;
> +			iomap->offset  = file_offset;
> +			iomap->length  = min_t(loff_t, len, chunk_remainder);
> +			iomap->length  = min_t(loff_t, iomap->length,
> +					       strip->ext_len - strip_offset);

[Severity: High]
Does this properly bound the physical mapping length?

The calculation restricts iomap->length to chunk_remainder and the strip's
length, but fails to restrict it to ext_size - local_offset. If a read/write
operation happens near the end of an interleaved extent where ext_size is not
a perfect multiple of the chunk size, the returned length could physically
exceed the logical boundaries of the extent.

> +			iomap->dax_dev = daxdev;
> +			iomap->type    = IOMAP_MAPPED;
> +			iomap->flags   = flags;

[Severity: Critical]
Does assigning the input operational flags directly to the output mapping
flags overwrite valid data?

The flags argument to iomap_begin contains input flags (e.g., IOMAP_WRITE
which is 1 << 0). In the iomap output namespace, 1 << 0 is IOMAP_F_NEW.
Setting IOMAP_F_NEW tells the DAX core that the memory was newly allocated
and requires zeroing on partial writes, overwriting valid existing data with
zeroes.

[ ... ]
> +static int
> +famfs_meta_to_dax_offset(struct inode *inode, struct iomap *iomap,
> +			 loff_t file_offset, off_t len, unsigned int flags)

[Severity: High]
Can this cause integer truncation on 32-bit architectures?

The len parameter is typed as off_t (which is 32-bit signed on 32-bit
architectures) instead of loff_t. A large I/O request could result in a
negative value, which is then sign-extended back to a 64-bit value in
min_t(loff_t, len, ...), resulting in a massive iomap->length and causing
out-of-bounds mappings. Should this match the iomap_begin signature with
loff_t?

> +{
> +	struct famfs_fs_info  *fsi = inode->i_sb->s_fs_info;
> +	struct famfs_file_meta *meta = inode->i_private;
> +	loff_t local_offset = file_offset;
> +	int rc;
> +	int i;
> +
> +	if (fsi->deverror || famfs_file_invalid(inode))
> +		goto err_out;
> +
> +	if (meta->fm_extent_type == FAMFS_IOC_EXT_INTERLEAVE)

[Severity: High]
Is there a Time-Of-Check to Time-Of-Use (TOCTOU) race condition here?

The local meta pointer is fetched from inode->i_private locklessly. It then
calls famfs_file_invalid(inode), which re-fetches inode->i_private. If
another thread initialized the file in between via FAMFSIOC_MAP_CREATE,
famfs_file_invalid() sees the new metadata and returns 0 (success). The first
thread then proceeds to dereference its locally cached meta pointer (which is
still NULL), crashing the kernel.

[ ... ]
> +		if (local_offset < dax_ext_len) {
> +			loff_t ext_len_remainder = dax_ext_len - local_offset;
> +			struct dax_device *daxdev;
> +
> +			daxdev = famfs_daxdev_from_index(fsi,
> +						meta->se[i].dev_index, &rc);
> +			if (!daxdev) {
> +				meta->error = true;
> +				return rc;
> +			}

[Severity: Critical]
Are we returning an uninitialized iomap structure here?

Because famfs_daxdev_from_index() returns a 0 error code when valid == 1,
famfs_meta_to_dax_offset() treats a NULL daxdev as a success (rc = 0) but
bails out early, returning 0 without initializing iomap->addr or
iomap->length. The DAX core would then operate on garbage physical addresses.

[ ... ]
> +			iomap->addr    = dax_ext_offset + local_offset;
> +			iomap->offset  = file_offset;
> +			iomap->length  = min_t(loff_t, len, ext_len_remainder);
> +			iomap->dax_dev = daxdev;
> +			iomap->type    = IOMAP_MAPPED;
> +			iomap->flags   = flags;

[Severity: Critical]
Does assigning the input operational flags directly to the output mapping
flags overwrite valid data?

The flags argument to iomap_begin contains input flags (e.g., IOMAP_WRITE
which is 1 << 0). In the iomap output namespace, 1 << 0 is IOMAP_F_NEW.
Setting IOMAP_F_NEW tells the DAX core that the memory was newly allocated
and requires zeroing on partial writes, overwriting valid existing data with
zeroes.

[ ... ]
> @@ -501,7 +793,7 @@ famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  		return rc;
>  	}
>  
> -	rc = dax_iomap_rw(iocb, from, NULL /*&famfs_iomap_ops*/);
> +	rc = dax_iomap_rw(iocb, from, &famfs_iomap_ops);
>  	inode_unlock(inode);
>  	return rc;
>  }

[Severity: High]
This is a pre-existing issue, but does famfs_dax_write_iter() bypass standard
VFS write checks?

It calls dax_iomap_rw() directly without invoking generic_write_checks() or
file_update_time(). This allows users to modify the contents of a setuid
binary without stripping the setuid bit, which is a classic privilege
escalation vector.


[Severity: Critical]
This is a pre-existing issue, but does FAMFSIOC_MAP_CREATE lack a privilege
check?

In famfs_file_init_dax() in fs/famfs/famfs_file.c:

	if (!famfs_opt_enabled(fsi, FAMFS_OPT_MAP_CREATE))
		return -EPERM;

Any unprivileged user with write access to a file could map and access
arbitrary physical memory on any registered DAX device. Should there be a
capable(CAP_SYS_ADMIN) or CAP_SYS_RAWIO check before parsing arbitrary physical
memory offsets? With the introduction of the iomap operations in this patch,
this allows the DAX core to perform I/O against unvalidated offsets.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=8

[PATCH V12 09/12] famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN)

[John Groves]

From: John Groves <John@Groves.net>

Famfs file maps (fmaps) may reference multiple daxdevs. Before passing
an fmap that references a new daxdev, the daxdev is pushed into the
kernel via FAMFSIOC_DAXDEV_OPEN). This adds daxdevs to daxdev_table for
index-based resolution from famfs extents to daxdevs.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_file.c            | 74 ++++++++++++++++++++++++++++++++
 include/uapi/linux/famfs_ioctl.h | 24 +++++++++++
 2 files changed, 98 insertions(+)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index e7f271ce6d03..e11a55ecf8d7 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -287,6 +287,76 @@ famfs_file_init_dax(struct file *file, void __user *arg)
 	return rc;
 }
 
+/**
+ * famfs_daxdev_open() - FAMFSIOC_DAXDEV_OPEN ioctl handler
+ * @file: any file in the famfs mount (the table is per-superblock)
+ * @arg:  ptr to struct famfs_ioc_daxdev in user space
+ *
+ * Register a devdax device (identified by path) into the mount's daxdev table
+ * at the caller-specified index, so files whose extents reference that index
+ * can be mapped. The path is resolved by lookup_daxdev() - the same helper the
+ * mount uses for the primary daxdev - so every slot is resolved identically.
+ * Registering exposes raw device memory, so it requires CAP_SYS_RAWIO.
+ */
+static int
+famfs_daxdev_open(struct file *file, void __user *arg)
+{
+	struct super_block *sb = file_inode(file)->i_sb;
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+	struct famfs_ioc_daxdev dd;
+	dev_t devno;
+	char *path;
+	int rc;
+
+	if (!capable(CAP_SYS_RAWIO))
+		return -EPERM;
+
+	if (copy_from_user(&dd, arg, sizeof(dd)))
+		return -EFAULT;
+
+	/* @flags is reserved; reject non-zero so it stays available */
+	if (dd.flags)
+		return -EINVAL;
+
+	/*
+	 * If this daxdev index is already populated there is nothing to do.
+	 * The index is cluster-invariant, so a valid slot already names this
+	 * device; skip the path resolution entirely. install_daxdev() rechecks
+	 * ->valid under the write lock, so this is purely an optimization.
+	 */
+	scoped_guard(rwsem_read, &fsi->devlist_sem) {
+		if (dd.daxdev_index >= fsi->dax_devlist->nslots)
+			return -EINVAL;
+		if (fsi->dax_devlist->devlist[dd.daxdev_index].valid)
+			return 0;
+	}
+
+	if (dd.daxdev_path_len == 0 || dd.daxdev_path_len >= PATH_MAX)
+		return -EINVAL;
+
+	/* +1 so the terminating NUL is included within the bound */
+	path = strndup_user((const char __user *)(uintptr_t)dd.daxdev_path,
+			    dd.daxdev_path_len + 1);
+	if (IS_ERR(path))
+		return PTR_ERR(path);
+
+	rc = lookup_daxdev(path, &devno);
+	if (rc)
+		goto out;
+
+	/*
+	 * The daxdev table is allocated at mount time (for the slot-0 primary),
+	 * so it is always present here; no need to allocate it.
+	 */
+	rc = famfs_install_daxdev(fsi, sb, dd.daxdev_index, devno, path);
+	if (rc)
+		pr_debug("%s: failed to install daxdev index %llu (%s)\n",
+		       __func__, dd.daxdev_index, path);
+out:
+	kfree(path);
+	return rc;
+}
+
 /**
  * famfs_file_ioctl() - Top-level famfs file ioctl handler
  * @file: the file
@@ -308,6 +378,10 @@ famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		rc = 0;
 		break;
 
+	case FAMFSIOC_DAXDEV_OPEN:
+		rc = famfs_daxdev_open(file, (void __user *)arg);
+		break;
+
 	case FAMFSIOC_MAP_CREATE:
 		rc = famfs_file_init_dax(file, (void __user *)arg);
 		break;
diff --git a/include/uapi/linux/famfs_ioctl.h b/include/uapi/linux/famfs_ioctl.h
index b4eb373c1ade..751d8b033c2e 100644
--- a/include/uapi/linux/famfs_ioctl.h
+++ b/include/uapi/linux/famfs_ioctl.h
@@ -77,6 +77,29 @@ struct famfs_ioc_fmap_header {
 	__u64 reserved1;
 };
 
+/**
+ * struct famfs_ioc_daxdev - register an additional backing daxdev by path
+ * @daxdev_index:    the (cluster-invariant) index this daxdev occupies in
+ *                   extent dev_index fields. Index 0 is the mount-time primary.
+ * @daxdev_path:     userspace pointer to the devdax device path (e.g.
+ *                   "/dev/dax0.0"); resolved in the kernel the same way the
+ *                   mount primary is.
+ * @daxdev_path_len: length of the path string, not counting the NUL.
+ * @flags:           reserved; must be zero.
+ *
+ * Standalone famfs registers every daxdev by path: the mount primary comes in
+ * as the mount device name, and slots 1..n come in here. (This deliberately
+ * differs from fuse's fd-based FUSE_DEV_IOC_DAXDEV_OPEN; each side is uniform
+ * within itself.) Passing the path by pointer keeps the struct fixed-size, so
+ * longer paths never require an ABI change.
+ */
+struct famfs_ioc_daxdev {
+	__u64 daxdev_index;
+	__u64 daxdev_path;
+	__u32 daxdev_path_len;
+	__u32 flags;
+};
+
 #define FAMFSIOC_MAGIC 'u'
 
 /* famfs file ioctl opcodes */
@@ -87,5 +110,6 @@ struct famfs_ioc_fmap_header {
  * famfs_ioc_fmap_header followed by the extent list (see above).
  */
 #define FAMFSIOC_MAP_CREATE    _IOW(FAMFSIOC_MAGIC,  0x51, struct famfs_ioc_fmap_header)
+#define FAMFSIOC_DAXDEV_OPEN   _IOW(FAMFSIOC_MAGIC,  0x52, struct famfs_ioc_daxdev)
 
 #endif /* FAMFS_IOCTL_H */
-- 
2.53.0

Re: [PATCH V12 09/12] famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Mixing goto-based cleanup with scope-based cleanup helpers in the same function.
--

--- Patch [9]: [PATCH V12 09/12] famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN) ---
commit 92fac6008c0bb8bb7ad31e3ae10289f93997e74b
Author: John Groves <John@Groves.net>

famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN)

This commit introduces the FAMFSIOC_DAXDEV_OPEN ioctl to register devdax
devices by path into the famfs mount's daxdev table. This allows files with
extents referencing these secondary devices to be mapped.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c

[ ... ]

> +static int
> +famfs_daxdev_open(struct file *file, void __user *arg)
> +{

[ ... ]

> +	scoped_guard(rwsem_read, &fsi->devlist_sem) {
> +		if (dd.daxdev_index >= fsi->dax_devlist->nslots)
> +			return -EINVAL;
> +		if (fsi->dax_devlist->devlist[dd.daxdev_index].valid)
> +			return 0;
> +	}
> +
> +	if (dd.daxdev_path_len == 0 || dd.daxdev_path_len >= PATH_MAX)
> +		return -EINVAL;
> +
> +	/* +1 so the terminating NUL is included within the bound */
> +	path = strndup_user((const char __user *)(uintptr_t)dd.daxdev_path,
> +			    dd.daxdev_path_len + 1);
> +	if (IS_ERR(path))
> +		return PTR_ERR(path);
> +
> +	rc = lookup_daxdev(path, &devno);
> +	if (rc)
> +		goto out;

[Severity: Low]
Does famfs_daxdev_open() mix goto-based cleanup with scope-based cleanup
helpers?

This function uses scoped_guard() earlier, but relies on a traditional goto
out label for manual memory cleanup of the path variable. The cleanup
subsystem guidelines recommend against mixing goto-based error handling and
scope-based cleanup in the same function to avoid confusing ownership
semantics.

Could path be declared with the __free() helper to eliminate the need for the
goto label entirely?

> +
> +	/*
> +	 * The daxdev table is allocated at mount time (for the slot-0 primary),
> +	 * so it is always present here; no need to allocate it.
> +	 */
> +	rc = famfs_install_daxdev(fsi, sb, dd.daxdev_index, devno, path);
> +	if (rc)
> +		pr_debug("%s: failed to install daxdev index %llu (%s)\n",
> +		       __func__, dd.daxdev_index, path);
> +out:
> +	kfree(path);
> +	return rc;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=9

[PATCH V12 10/12] famfs: Add runtime operation-permission (opts) framework

[John Groves]

From: John Groves <John@Groves.net>

famfs denies most namespace, attribute and data operations by default
because the userspace log, not the kernel, is authoritative for a famfs
instance. Earlier commits already guard each such operation with a
famfs_opt_enabled(fsi, FAMFS_OPT_x) check backed by a permissive stub. This
commit defines the permission bitmap and makes those checks live.

Add:
 - FAMFS_OPT_* (uapi): a u64 permission bitmap, one bit per gated operation
   (create, mkdir, mknod, symlink, link, unlink, rmdir, rename, the four
   setattr components, data write, and MAP_CREATE), plus FAMFS_OPT_ALL. The
   FAMFS_OPT_XATTR bit is reserved - famfs has no xattr ops yet.
 - fsi->opts: a per-mount atomic64 bitmap initialized to FAMFS_OPT_DEFAULT,
   which sets famfs's default policy: create, mkdir, chmod, chown, utimes,
   write and MAP_CREATE are permitted; unlink of mapped files, link,
   symlink, mknod, rmdir, rename and truncate are denied.
 - the real famfs_opt_enabled() (replacing the stub), so every planted gate
   now consults fsi->opts.
 - FAMFSIOC_{GET,SET,CLEAR}_OPTS: read the bitmap, or enable/disable the
   bits set in a caller-supplied mask, returning the resulting bitmap.
   SET/CLEAR require CAP_SYS_ADMIN and reject unknown bits with -EINVAL;
   the bitmap is updated with atomic RMW so the checks stay lockless.
Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_file.c            | 55 ++++++++++++++++++++++++++++++++
 fs/famfs/famfs_inode.c           |  1 +
 fs/famfs/famfs_internal.h        | 30 ++++++++++++++---
 include/uapi/linux/famfs_ioctl.h | 45 ++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index e11a55ecf8d7..abf049b32a4b 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -357,6 +357,49 @@ famfs_daxdev_open(struct file *file, void __user *arg)
 	return rc;
 }
 
+/**
+ * famfs_get_opts() - FAMFSIOC_GET_OPTS: return the permission bitmap
+ */
+static long famfs_get_opts(struct famfs_fs_info *fsi, void __user *arg)
+{
+	struct famfs_ioc_opts o = { .opts = atomic64_read(&fsi->opts) };
+
+	if (copy_to_user(arg, &o, sizeof(o)))
+		return -EFAULT;
+
+	return 0;
+}
+
+/*
+ * famfs_modify_opts() - FAMFSIOC_SET_OPTS / FAMFSIOC_CLEAR_OPTS
+ * @set: true to enable (OR in) the requested bits, false to disable (mask out)
+ *
+ * The caller supplies a mask of FAMFS_OPT_* bits; the resulting bitmap is
+ * returned. Requires CAP_SYS_ADMIN since it changes mount-wide policy.
+ */
+static long famfs_modify_opts(struct famfs_fs_info *fsi, void __user *arg,
+			      bool set)
+{
+	struct famfs_ioc_opts o;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+	if (copy_from_user(&o, arg, sizeof(o)))
+		return -EFAULT;
+	if (o.opts & ~FAMFS_OPT_ALL)
+		return -EINVAL;
+
+	if (set)
+		o.opts = atomic64_fetch_or(o.opts, &fsi->opts) | o.opts;
+	else
+		o.opts = atomic64_fetch_and(~o.opts, &fsi->opts) & ~o.opts;
+
+	if (copy_to_user(arg, &o, sizeof(o)))
+		return -EFAULT;
+
+	return 0;
+}
+
 /**
  * famfs_file_ioctl() - Top-level famfs file ioctl handler
  * @file: the file
@@ -378,6 +421,18 @@ famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		rc = 0;
 		break;
 
+	case FAMFSIOC_GET_OPTS:
+		rc = famfs_get_opts(fsi, (void __user *)arg);
+		break;
+
+	case FAMFSIOC_SET_OPTS:
+		rc = famfs_modify_opts(fsi, (void __user *)arg, true);
+		break;
+
+	case FAMFSIOC_CLEAR_OPTS:
+		rc = famfs_modify_opts(fsi, (void __user *)arg, false);
+		break;
+
 	case FAMFSIOC_DAXDEV_OPEN:
 		rc = famfs_daxdev_open(file, (void __user *)arg);
 		break;
diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index a6c3b4574e69..6cbd7d657fd8 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -717,6 +717,7 @@ static int famfs_init_fs_context(struct fs_context *fc)
 		return -ENOMEM;
 
 	init_rwsem(&fsi->devlist_sem);
+	atomic64_set(&fsi->opts, FAMFS_OPT_DEFAULT);
 	fsi->mount_opts.mode = FAMFS_DEFAULT_MODE;
 	fc->s_fs_info        = fsi;
 	fc->ops              = &famfs_context_ops;
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index b5f9c8d0349f..26873162b4a0 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -12,11 +12,24 @@
 #define FAMFS_INTERNAL_H
 
 #include <linux/rwsem.h>
+#include <linux/atomic.h>
 #include <linux/bits.h>
 #include <linux/build_bug.h>
 
 #include <linux/famfs_ioctl.h>
 
+/*
+ * Default operation-permission bitmap (see FAMFS_OPT_* in the uapi header).
+ * This preserves famfs's historical behavior: file/dir creation, the fmap
+ * ioctl, data writes, and the non-resize setattr components are permitted;
+ * unlink of mapped files, link, symlink, mknod, rmdir, rename and truncate
+ * are denied until enabled via FAMFSIOC_SET_OPTS.
+ */
+#define FAMFS_OPT_DEFAULT	(FAMFS_OPT_CREATE | FAMFS_OPT_MKDIR | \
+				 FAMFS_OPT_CHMOD | FAMFS_OPT_CHOWN | \
+				 FAMFS_OPT_UTIMES | FAMFS_OPT_WRITE | \
+				 FAMFS_OPT_MAP_CREATE)
+
 extern const struct file_operations famfs_file_operations;
 
 /*
@@ -104,6 +117,8 @@ struct famfs_dax_devlist {
  * @famfs_fs_info
  *
  * @mount_opts:  The mount options
+ * @opts:        Operation-permission bitmap (FAMFS_OPT_*), adjusted at runtime
+ *               via the FAMFSIOC_{GET,SET,CLEAR}_OPTS ioctls
  * @deverror:    True if the dax device has called our notify_failure entry
  *               point, or if other "shutdown" conditions exist
  * @dax_devlist: Table of backing daxdevs (slot 0 is the mount primary)
@@ -111,16 +126,23 @@ struct famfs_dax_devlist {
  */
 struct famfs_fs_info {
 	struct famfs_mount_opts   mount_opts;
+	atomic64_t                opts;
 	bool                      deverror;
 	struct famfs_dax_devlist *dax_devlist;
 	struct rw_semaphore       devlist_sem;
 };
 
-/* This stub will be replaced in a later commit 
- * Note: the opt parameter is intentionally unused, and will be used by
- * the replacement function when that commit lands
+/*
+ * famfs_opt_enabled() - is operation permission @opt enabled for this mount?
+ *
+ * @opt is a single FAMFS_OPT_* bit; returns true if that operation is
+ * permitted. The bitmap is read locklessly (updated via atomic RMW by the
+ * FAMFSIOC_{SET,CLEAR}_OPTS ioctls).
  */
-#define famfs_opt_enabled(fsi, opt) (fsi != 0)
+static inline bool famfs_opt_enabled(struct famfs_fs_info *fsi, u64 opt)
+{
+	return !!(atomic64_read(&fsi->opts) & opt);
+}
 
 int lookup_daxdev(const char *pathname, dev_t *devno);
 int famfs_devlist_alloc(struct famfs_fs_info *fsi);
diff --git a/include/uapi/linux/famfs_ioctl.h b/include/uapi/linux/famfs_ioctl.h
index 751d8b033c2e..efe6ef263975 100644
--- a/include/uapi/linux/famfs_ioctl.h
+++ b/include/uapi/linux/famfs_ioctl.h
@@ -100,6 +100,48 @@ struct famfs_ioc_daxdev {
 	__u32 flags;
 };
 
+/*
+ * Mount-wide operation permissions, queried and modified via the
+ * FAMFSIOC_{GET,SET,CLEAR}_OPTS ioctls. A set bit means the operation is
+ * permitted; a clear bit means it is rejected with -EPERM. famfs denies most
+ * of these by default because the userspace log, not the kernel, is
+ * authoritative for a famfs instance.
+ */
+#define FAMFS_OPT_CREATE	(1ULL << 0)  /* create a regular file        */
+#define FAMFS_OPT_MKDIR		(1ULL << 1)  /* mkdir                        */
+#define FAMFS_OPT_MKNOD		(1ULL << 2)  /* mknod a special file         */
+#define FAMFS_OPT_SYMLINK	(1ULL << 3)  /* create a symlink             */
+#define FAMFS_OPT_LINK		(1ULL << 4)  /* hard link                    */
+#define FAMFS_OPT_UNLINK	(1ULL << 5)  /* unlink a mapped file         */
+#define FAMFS_OPT_RMDIR		(1ULL << 6)  /* rmdir                        */
+#define FAMFS_OPT_RENAME	(1ULL << 7)  /* rename                       */
+#define FAMFS_OPT_CHMOD		(1ULL << 8)  /* setattr ATTR_MODE            */
+#define FAMFS_OPT_CHOWN		(1ULL << 9)  /* setattr ATTR_UID / ATTR_GID  */
+#define FAMFS_OPT_TRUNCATE	(1ULL << 10) /* setattr ATTR_SIZE (resize)   */
+#define FAMFS_OPT_UTIMES	(1ULL << 11) /* setattr ATTR_ATIME/ATTR_MTIME*/
+#define FAMFS_OPT_WRITE		(1ULL << 12) /* write file data              */
+#define FAMFS_OPT_XATTR		(1ULL << 13) /* set/remove xattrs (reserved) */
+#define FAMFS_OPT_MAP_CREATE	(1ULL << 14) /* attach an fmap (MAP_CREATE)  */
+
+#define FAMFS_OPT_ALL		(FAMFS_OPT_CREATE | FAMFS_OPT_MKDIR | \
+				 FAMFS_OPT_MKNOD | FAMFS_OPT_SYMLINK | \
+				 FAMFS_OPT_LINK | FAMFS_OPT_UNLINK | \
+				 FAMFS_OPT_RMDIR | FAMFS_OPT_RENAME | \
+				 FAMFS_OPT_CHMOD | FAMFS_OPT_CHOWN | \
+				 FAMFS_OPT_TRUNCATE | FAMFS_OPT_UTIMES | \
+				 FAMFS_OPT_WRITE | FAMFS_OPT_XATTR | \
+				 FAMFS_OPT_MAP_CREATE)
+
+/**
+ * struct famfs_ioc_opts - operation-permission bitmap
+ * @opts: for GET, the current bitmap is returned here. For SET/CLEAR, the
+ *        caller-supplied mask of bits to enable/disable on input, and the
+ *        resulting bitmap on return.
+ */
+struct famfs_ioc_opts {
+	__u64 opts;
+};
+
 #define FAMFSIOC_MAGIC 'u'
 
 /* famfs file ioctl opcodes */
@@ -111,5 +153,8 @@ struct famfs_ioc_daxdev {
  */
 #define FAMFSIOC_MAP_CREATE    _IOW(FAMFSIOC_MAGIC,  0x51, struct famfs_ioc_fmap_header)
 #define FAMFSIOC_DAXDEV_OPEN   _IOW(FAMFSIOC_MAGIC,  0x52, struct famfs_ioc_daxdev)
+#define FAMFSIOC_GET_OPTS      _IOR(FAMFSIOC_MAGIC,  0x53, struct famfs_ioc_opts)
+#define FAMFSIOC_SET_OPTS      _IOWR(FAMFSIOC_MAGIC, 0x54, struct famfs_ioc_opts)
+#define FAMFSIOC_CLEAR_OPTS    _IOWR(FAMFSIOC_MAGIC, 0x55, struct famfs_ioc_opts)
 
 #endif /* FAMFS_IOCTL_H */
-- 
2.53.0

Re: [PATCH V12 10/12] famfs: Add runtime operation-permission (opts) framework

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mprotect() can bypass FAMFS_OPT_WRITE restrictions because mmap only checks VM_WRITE.
- [High] rename() bypasses FAMFS_OPT_UNLINK and FAMFS_OPT_RMDIR restrictions for target files and directories.
--

--- Patch [10]: [PATCH V12 10/12] famfs: Add runtime operation-permission (opts) framework ---
commit e932914ca4460e6b742e80deb76ae5e6b7bac595
Author: John Groves <John@Groves.net>

famfs: Add runtime operation-permission (opts) framework

famfs denies most namespace, attribute and data operations by default
because the userspace log, not the kernel, is authoritative for a famfs
instance. Earlier commits already guard each such operation with a
famfs_opt_enabled(fsi, FAMFS_OPT_x) check backed by a permissive stub. This
commit defines the permission bitmap and makes those checks live.

> diff --git a/include/uapi/linux/famfs_ioctl.h b/include/uapi/linux/famfs_ioctl.h
> index 751d8b033c2ee..efe6ef2639759 100644
> --- a/include/uapi/linux/famfs_ioctl.h
> +++ b/include/uapi/linux/famfs_ioctl.h
> @@ -100,6 +100,48 @@ struct famfs_ioc_daxdev {
>  	__u32 flags;
>  };
>  
> +/*
> + * Mount-wide operation permissions, queried and modified via the
> + * FAMFSIOC_{GET,SET,CLEAR}_OPTS ioctls. A set bit means the operation is
> + * permitted; a clear bit means it is rejected with -EPERM. famfs denies most
> + * of these by default because the userspace log, not the kernel, is
> + * authoritative for a famfs instance.
> + */
> +#define FAMFS_OPT_CREATE	(1ULL << 0)  /* create a regular file        */
> +#define FAMFS_OPT_MKDIR		(1ULL << 1)  /* mkdir                        */
> +#define FAMFS_OPT_MKNOD		(1ULL << 2)  /* mknod a special file         */
> +#define FAMFS_OPT_SYMLINK	(1ULL << 3)  /* create a symlink             */
> +#define FAMFS_OPT_LINK		(1ULL << 4)  /* hard link                    */
> +#define FAMFS_OPT_UNLINK	(1ULL << 5)  /* unlink a mapped file         */
> +#define FAMFS_OPT_RMDIR		(1ULL << 6)  /* rmdir                        */
> +#define FAMFS_OPT_RENAME	(1ULL << 7)  /* rename                       */

[Severity: High]
Does this bypass the FAMFS_OPT_UNLINK and FAMFS_OPT_RMDIR policy restrictions
for target files and directories?

If FAMFS_OPT_RENAME is enabled but FAMFS_OPT_UNLINK is disabled, a user can
rename a file over an existing target mapped file.

In fs/famfs/famfs_inode.c:famfs_rename():

	if (!famfs_opt_enabled(fsi, FAMFS_OPT_RENAME))
		return -EPERM;

	return simple_rename(idmap, old_dir, old_dentry, new_dir, new_dentry,
			     flags);

This delegates to simple_rename(), which silently replaces the target by
calling simple_unlink() (or simple_rmdir()) on it. This directly drops the
inode link count and bypasses the filesystem's famfs_unlink() and
famfs_rmdir() hooks, completely avoiding the FAMFS_OPT_UNLINK check.

> +#define FAMFS_OPT_CHMOD		(1ULL << 8)  /* setattr ATTR_MODE            */
> +#define FAMFS_OPT_CHOWN		(1ULL << 9)  /* setattr ATTR_UID / ATTR_GID  */
> +#define FAMFS_OPT_TRUNCATE	(1ULL << 10) /* setattr ATTR_SIZE (resize)   */
> +#define FAMFS_OPT_UTIMES	(1ULL << 11) /* setattr ATTR_ATIME/ATTR_MTIME*/
> +#define FAMFS_OPT_WRITE		(1ULL << 12) /* write file data              */

[Severity: High]
Does this allow mprotect() to bypass FAMFS_OPT_WRITE restrictions and obtain a
writable memory mapping to a file when it should be read-only?

In fs/famfs/famfs_file.c:famfs_file_mmap():

	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE) &&
	    !famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))
		return -EPERM;

The permission check only validates VM_WRITE. If a user calls mmap with
PROT_READ | MAP_SHARED, it sets VM_MAYWRITE but not VM_WRITE, bypassing the
FAMFS_OPT_WRITE check. The user could then call mprotect(PROT_WRITE) to
upgrade the mapping to writable without any filesystem callbacks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=10

[PATCH V12 11/12] famfs: Report device capacity via statfs so df works

[John Groves]

From: John Groves <John@Groves.net>

Replace simple_statfs(), which reports zero blocks (so df omits the mount),
with famfs_statfs() reporting real capacity and usage.

Add dax_fsdev_size() in drivers/dax/fsdev.c, returning the size fsdev
caches at probe (dev_dax->cached_size - the sum of the device's ranges,
stable while bound), exported. It lives in fsdev.c because cached_size is
set only by the fsdev driver, and famfs only ever holds fsdev-mode daxdevs
(fs_dax_get() enforces DAXDRV_FSDEV_TYPE); famfs.ko therefore depends on
fsdev_dax.ko.

famfs tracks two byte counters under a new stats_sem:
 - total_capacity: summed in famfs_install_daxdev() from dax_fsdev_size(),
   covering the mount primary and every DAXDEV_OPEN secondary, counted once
   per daxdev (on the valid 0->1 transition).
 - used_capacity: summed in famfs_file_init_dax() from the fmap's mapped
   device bytes (superblock + log + data files).

famfs_statfs() reports total and free (total - used). Free is an
approximation of the userspace allocator's free space (it ignores allocator
gaps and reserved regions), which is adequate for df.

(Side note: I am the maintainer of drivers/dax/fsdev.c)

Signed-off-by: John Groves <john@groves.net>
---
 drivers/dax/fsdev.c       | 19 +++++++++++++++++
 fs/famfs/famfs_file.c     |  5 +++++
 fs/famfs/famfs_inode.c    | 43 ++++++++++++++++++++++++++++++++++++++-
 fs/famfs/famfs_internal.h |  8 ++++++++
 include/linux/dax.h       |  1 +
 5 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
index 188b2526bee4..a5b4b2d79428 100644
--- a/drivers/dax/fsdev.c
+++ b/drivers/dax/fsdev.c
@@ -104,6 +104,25 @@ static size_t fsdev_dax_recovery_write(struct dax_device *dax_dev, pgoff_t pgoff
 	return _copy_from_iter_flushcache(addr, bytes, i);
 }
 
+/**
+ * dax_fsdev_size() - total size in bytes of an fsdev dax device
+ * @dax_dev: the dax device (must be bound to this driver)
+ *
+ * Returns the size cached at probe time (sum of all ranges); it cannot change
+ * while the driver is bound. Only valid for fsdev dax devices - callers
+ * ensure that (e.g. fs_dax_get() enforces DAXDRV_FSDEV_TYPE). Returns 0 if the
+ * device is not alive.
+ */
+u64 dax_fsdev_size(struct dax_device *dax_dev)
+{
+	struct dev_dax *dev_dax = dax_get_private(dax_dev);
+
+	if (!dev_dax)
+		return 0;
+	return dev_dax->cached_size;
+}
+EXPORT_SYMBOL_GPL(dax_fsdev_size);
+
 static const struct dax_operations dev_dax_ops = {
 	.direct_access = fsdev_dax_direct_access,
 	.zero_page_range = fsdev_dax_zero_page_range,
diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index abf049b32a4b..5be39d677089 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -280,6 +280,11 @@ famfs_file_init_dax(struct file *file, void __user *arg)
 	}
 	inode_unlock(inode);
 
+	/* Account the mapped device bytes for statfs (only on success) */
+	if (!rc) {
+		scoped_guard(rwsem_write, &fsi->stats_sem)
+			fsi->used_capacity += extent_total;
+	}
 out:
 	kvfree(fmap_buf);
 	if (meta)
diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index 6cbd7d657fd8..3c0d1094d653 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -24,6 +24,7 @@
 #include <linux/iomap.h>
 #include <linux/path.h>
 #include <linux/namei.h>
+#include <linux/statfs.h>
 
 #include "famfs_internal.h"
 
@@ -307,8 +308,37 @@ static void famfs_evict_inode(struct inode *inode)
 	clear_inode(inode);
 }
 
+/*
+ * famfs_statfs() - report device capacity and consumption so 'df' works.
+ * @total_capacity is the sum of installed daxdev sizes; @used_capacity is the
+ * sum of device bytes mapped by fmaps (superblock + log + data files). Free is
+ * the difference - an approximation of the userspace allocator's free space
+ * (it ignores allocator gaps / reserved regions), which is fine for df.
+ */
+static int famfs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+	struct famfs_fs_info *fsi = dentry->d_sb->s_fs_info;
+	u64 total, used, free;
+
+	scoped_guard(rwsem_read, &fsi->stats_sem) {
+		total = fsi->total_capacity;
+		used  = fsi->used_capacity;
+	}
+	free = total > used ? total - used : 0;
+
+	buf->f_type    = FAMFS_SUPER_MAGIC;
+	buf->f_bsize   = PAGE_SIZE;
+	buf->f_frsize  = PAGE_SIZE;
+	buf->f_blocks  = total >> PAGE_SHIFT;
+	buf->f_bfree   = free  >> PAGE_SHIFT;
+	buf->f_bavail  = free  >> PAGE_SHIFT;	/* no root reservation */
+	buf->f_namelen = NAME_MAX;
+	buf->f_fsid    = u64_to_fsid(huge_encode_dev(dentry->d_sb->s_dev));
+	return 0;
+}
+
 static const struct super_operations famfs_super_ops = {
-	.statfs		= simple_statfs,
+	.statfs		= famfs_statfs,
 	.drop_inode	= inode_just_drop,
 	.show_options	= famfs_show_options,
 	.evict_inode    = famfs_evict_inode,
@@ -399,6 +429,7 @@ int famfs_install_daxdev(
 		const char *name)
 {
 	struct famfs_daxdev *daxdev;
+	struct dax_device *devp = NULL;
 	int rc = 0;
 
 	if (index >= fsi->dax_devlist->nslots) {
@@ -462,6 +493,15 @@ int famfs_install_daxdev(
 
 		wmb(); /* All other fields must be visible before valid */
 		daxdev->valid = 1;
+		devp = daxdev->devp;
+	}
+
+	/* Freshly installed: add its capacity to the statfs accounting */
+	if (devp) {
+		u64 sz = dax_fsdev_size(devp);
+
+		scoped_guard(rwsem_write, &fsi->stats_sem)
+			fsi->total_capacity += sz;
 	}
 
 	return 0;
@@ -717,6 +757,7 @@ static int famfs_init_fs_context(struct fs_context *fc)
 		return -ENOMEM;
 
 	init_rwsem(&fsi->devlist_sem);
+	init_rwsem(&fsi->stats_sem);
 	atomic64_set(&fsi->opts, FAMFS_OPT_DEFAULT);
 	fsi->mount_opts.mode = FAMFS_DEFAULT_MODE;
 	fc->s_fs_info        = fsi;
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index 26873162b4a0..0bf50774fa82 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -123,6 +123,11 @@ struct famfs_dax_devlist {
  *               point, or if other "shutdown" conditions exist
  * @dax_devlist: Table of backing daxdevs (slot 0 is the mount primary)
  * @devlist_sem: Serializes installs into, and teardown of, @dax_devlist
+ * @stats_sem:   Protects the statfs accounting counters below
+ * @total_capacity: Sum of installed daxdev sizes, in bytes (grows as daxdevs
+ *               are added)
+ * @used_capacity:  Sum of installed fmap sizes, in bytes (grows as MAP_CREATE
+ *               attaches fmaps; this is device bytes consumed, not file size)
  */
 struct famfs_fs_info {
 	struct famfs_mount_opts   mount_opts;
@@ -130,6 +135,9 @@ struct famfs_fs_info {
 	bool                      deverror;
 	struct famfs_dax_devlist *dax_devlist;
 	struct rw_semaphore       devlist_sem;
+	struct rw_semaphore       stats_sem;
+	u64                       total_capacity;
+	u64                       used_capacity;
 };
 
 /*
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 29113eb95e72..c25ef499d04f 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -256,6 +256,7 @@ static inline void dax_break_layout_final(struct inode *inode)
 
 bool dax_alive(struct dax_device *dax_dev);
 void *dax_get_private(struct dax_device *dax_dev);
+u64 dax_fsdev_size(struct dax_device *dax_dev);
 int dax_set_ops(struct dax_device *dax_dev, const struct dax_operations *ops);
 long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
 		enum dax_access_mode mode, void **kaddr, unsigned long *pfn);
-- 
2.53.0

Re: [PATCH V12 11/12] famfs: Report device capacity via statfs so df works

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `used_capacity` counter is permanently leaked when mapped files are unlinked or their inodes are evicted.
- [High] dax_fsdev_size() accesses the private dev_dax structure without holding the required dax_read_lock(), leading to a use-after-free race condition with concurrent device unbinds.
- [High] Missing read memory barrier when locklessly checking `daxdev->valid` allows reading an uninitialized or stale `daxdev->devp` pointer.
- [Low] famfs_file_init_dax() mixes goto-based error handling with scope-based cleanup helpers, violating the subsystem rules.
--

--- Patch [11]: [PATCH V12 11/12] famfs: Report device capacity via statfs so df works ---
commit 72bb530cad0a65ccec5c1eea93f055e5015f64ec
Author: John Groves <John@Groves.net>

famfs: Report device capacity via statfs so df works

Replace simple_statfs(), which reports zero blocks (so df omits the mount),
with famfs_statfs() reporting real capacity and usage.

Add dax_fsdev_size() in drivers/dax/fsdev.c, returning the size fsdev
caches at probe (dev_dax->cached_size - the sum of the device's ranges,
stable while bound), exported. It lives in fsdev.c because cached_size is
set only by the fsdev driver, and famfs only ever holds fsdev-mode daxdevs
(fs_dax_get() enforces DAXDRV_FSDEV_TYPE); famfs.ko therefore depends on
fsdev_dax.ko.

famfs tracks two byte counters under a new stats_sem:
 - total_capacity: summed in famfs_install_daxdev() from dax_fsdev_size(),
   covering the mount primary and every DAXDEV_OPEN secondary, counted once
   per daxdev (on the valid 0->1 transition).
 - used_capacity: summed in famfs_file_init_dax() from the fmap's mapped
   device bytes (superblock + log + data files).

famfs_statfs() reports total and free (total - used). Free is an
approximation of the userspace allocator's free space (it ignores allocator
gaps and reserved regions), which is adequate for df.

(Side note: I am the maintainer of drivers/dax/fsdev.c)

Signed-off-by: John Groves <john@groves.net>

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index 188b2526bee45..a5b4b2d794286 100644
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -104,6 +104,25 @@ static size_t fsdev_dax_recovery_write(struct dax_device *dax_dev, pgoff_t pgoff
>  	return _copy_from_iter_flushcache(addr, bytes, i);
>  }
>  
> +/**
> + * dax_fsdev_size() - total size in bytes of an fsdev dax device
> + * @dax_dev: the dax device (must be bound to this driver)
> + *
> + * Returns the size cached at probe time (sum of all ranges); it cannot change
> + * while the driver is bound. Only valid for fsdev dax devices - callers
> + * ensure that (e.g. fs_dax_get() enforces DAXDRV_FSDEV_TYPE). Returns 0 if the
> + * device is not alive.
> + */
> +u64 dax_fsdev_size(struct dax_device *dax_dev)
> +{
> +	struct dev_dax *dev_dax = dax_get_private(dax_dev);
> +
> +	if (!dev_dax)
> +		return 0;
> +	return dev_dax->cached_size;
> +}

[Severity: High]
Does dax_get_private() require holding the dax_read_lock() here?

In the DAX subsystem, accessing dev_dax from a dax_device requires holding the
SRCU-based dax_read_lock(), because a concurrent driver unbind can call
kill_dax(), which clears DAXDEV_ALIVE, calls synchronize_srcu(&dax_srcu), and
frees the dev_dax structure.

Since dax_fsdev_size() and its caller famfs_install_daxdev() do not hold
dax_read_lock(), could a concurrent unbind result in dev_dax being freed
before dev_dax->cached_size is dereferenced?

> +EXPORT_SYMBOL_GPL(dax_fsdev_size);
> +
>  static const struct dax_operations dev_dax_ops = {
> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index abf049b32a4b9..5be39d6770899 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
> @@ -280,6 +280,11 @@ famfs_file_init_dax(struct file *file, void __user *arg)
>  	}
>  	inode_unlock(inode);
>  
> +	/* Account the mapped device bytes for statfs (only on success) */
> +	if (!rc) {
> +		scoped_guard(rwsem_write, &fsi->stats_sem)
> +			fsi->used_capacity += extent_total;

[Severity: High]
Is fsi->used_capacity ever decremented when files are unlinked?

If famfs_file_init_dax() increments fsi->used_capacity here, should there be a
matching decrement path when the mapping is torn down and its private metadata
is freed in famfs_evict_inode()?

If FAMFS_OPT_UNLINK is enabled via ioctl, userspace could repeatedly create,
map, and delete files, causing used_capacity to monotonically grow until it
exceeds total_capacity. Would this permanently break the statfs free space
accounting for the mount?

> +	}
>  out:
>  	kvfree(fmap_buf);
>  	if (meta)
>  		famfs_meta_free(meta);

[Severity: Low]
This isn't a bug, but does this code mix goto-based error handling with
scope-based cleanup helpers?

Using the out label for manual cleanup (freeing fmap_buf and meta) while also
using scoped_guard() in the same function violates the documented cleanup
guidelines, which state that functions must not mix goto-based cleanup labels
and __free()/guard() declarations.

> diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
> index 6cbd7d657fd80..3c0d1094d6532 100644
> --- a/fs/famfs/famfs_inode.c
> +++ b/fs/famfs/famfs_inode.c
> @@ -462,6 +493,15 @@ int famfs_install_daxdev(
>  
>  		wmb(); /* All other fields must be visible before valid */
>  		daxdev->valid = 1;
> +		devp = daxdev->devp;
> +	}

[Severity: High]
Is there a matching read memory barrier on the consumer side?

Here the producer uses wmb() before setting daxdev->valid = 1 to make the new
devp visible.

On the IO/fault path, famfs_daxdev_from_index() calls famfs_dax_err() to
locklessly read dd->valid, and if true, proceeds to read dd->devp:

fs/famfs/famfs_file.c:famfs_dax_err() {
    if (!dd->valid) {
        ...
}

fs/famfs/famfs_file.c:famfs_daxdev_from_index() {
    ...
    *errp = 0;
    return dd->devp;
}

If there is no paired read memory barrier (e.g., smp_rmb() or
smp_load_acquire()) on the consumer side, could the CPU reorder the loads?

A concurrent thread might observe valid == 1 but still read a stale or NULL
value for devp, resulting in a NULL pointer dereference during IO.

> +
> +	/* Freshly installed: add its capacity to the statfs accounting */
> +	if (devp) {
> +		u64 sz = dax_fsdev_size(devp);
> +
> +		scoped_guard(rwsem_write, &fsi->stats_sem)
> +			fsi->total_capacity += sz;
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000000@email.amazonses.com?part=11

[PATCH V12 12/12] famfs: Add documentation

[John Groves]

From: John Groves <John@Groves.net>

Add Documentation/filesystems/famfs.rst and update MAINTAINERS

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
 Documentation/filesystems/famfs.rst | 142 ++++++++++++++++++++++++++++
 Documentation/filesystems/index.rst |   1 +
 MAINTAINERS                         |   1 +
 3 files changed, 144 insertions(+)
 create mode 100644 Documentation/filesystems/famfs.rst

diff --git a/Documentation/filesystems/famfs.rst b/Documentation/filesystems/famfs.rst
new file mode 100644
index 000000000000..048791bcbcfd
--- /dev/null
+++ b/Documentation/filesystems/famfs.rst
@@ -0,0 +1,142 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _famfs_index:
+
+==================================================================
+famfs: The fabric-attached memory file system
+==================================================================
+
+- Copyright (C) 2024-2026 Micron Technology, Inc.
+
+Introduction
+============
+Compute Express Link (CXL) provides a mechanism for disaggregated or
+fabric-attached memory (FAM). This creates opportunities for data sharing;
+clustered apps that would otherwise have to shard or replicate data can
+share one copy in disaggregated memory.
+
+Famfs, which is not CXL-specific in any way, provides a mechanism for
+multiple hosts to concurrently access data in shared memory, by giving it
+a file system interface. With famfs, any app that understands files can
+access data sets in shared memory. Although famfs supports read and write,
+the real point is to support mmap, which provides direct (dax) access to
+the memory - either writable or read-only.
+
+Shared memory can pose complex coherency and synchronization issues, but
+there are also simple cases. Two simple and eminently useful patterns that
+occur frequently in data analytics and AI are:
+
+* Serial Sharing - Only one host or process at a time has access to a file
+* Read-only Sharing - Multiple hosts or processes share read-only access
+  to a file
+
+The famfs file system is part of the famfs framework; user space
+components [1] handle metadata allocation and distribution, and populate
+the in-kernel files (via ioctls) so that they map directly to shared
+memory.
+
+The famfs framework manages coherency of its own metadata and structures,
+but does not attempt to manage coherency for applications.
+
+Famfs also provides data isolation between files. That is, even though
+the host has access to an entire memory "device" (as a devdax device), apps
+cannot write to memory for which the file is read-only, and mapping one
+file provides isolation from the memory of all other files. This is pretty
+basic, but some experimental shared memory usage patterns provide no such
+isolation.
+
+Principles of Operation
+=======================
+
+Famfs is a file system with one or more devdax devices as a first-class
+backing device(s). Metadata maintenance and query operations happen
+entirely in user space.
+
+The famfs user space provides each file's map (fmap) and its backing
+devdax devices to the kernel through ioctls (FAMFSIOC_MAP_CREATE and
+FAMFSIOC_DAXDEV_OPEN), after which read/write/mapping faults are handled
+entirely in the kernel with no up-calls for all active files.
+
+The famfs user space is responsible for maintaining and distributing
+consistent metadata. This is currently handled via an append-only
+metadata log within the memory, but this is orthogonal to the famfs
+kernel code.
+
+Once instantiated, "the same file" on each host points to the same shared
+memory, but in-memory metadata (inodes, etc.) is ephemeral on each host
+that has a famfs instance mounted. Use cases are free to allow or not
+allow mutations to data on a file-by-file basis.
+
+When an app accesses a data object in a famfs file, there is no page cache
+involvement. The CPU cache is loaded directly from the shared memory. In
+some use cases, this is an enormous reduction in read amplification
+compared to loading an entire page into the page cache.
+
+
+Famfs is Not a Conventional File System
+---------------------------------------
+
+Famfs files can be accessed by conventional means, but there are
+limitations. The famfs kernel module is not involved in the allocation of
+backing memory for files at all; the famfs user space creates files and
+supplies their fmaps and devdax device info to the kernel via ioctls.
+
+Famfs differs in some important ways from conventional file systems:
+
+* Files must be pre-allocated by the famfs framework; allocation is never
+  performed on (or after) write.
+* Any operation that changes a file's size is considered to put the file
+  in an invalid state, disabling access to the data. It may be possible to
+  revisit this in the future. (Typically the famfs user space can restore
+  files to a valid state by replaying the famfs metadata log.)
+
+Famfs exists to apply the existing file system abstractions to shared
+memory so applications and workflows can more easily adapt to an
+environment with disaggregated shared memory.
+
+Memory Error Handling
+=====================
+
+Possible memory errors include timeouts, poison, and unexpected
+reconfiguration of an underlying dax device. In all of these cases, famfs
+receives a call from the devdax layer via its
+dax_holder_operations->notify_failure() function. If any memory errors have
+been detected, access to the affected
+daxdev is disabled to avoid further errors or corruption.
+
+In all known cases, famfs can be unmounted cleanly. In most cases errors
+can be cleared by re-initializing the memory - at which point a new famfs
+file system can be created.
+
+Key Requirements
+================
+
+The primary requirements for famfs are:
+
+1. Must support a file system abstraction backed by sharable devdax memory
+2. Files must efficiently handle VMA faults
+3. Must support metadata distribution in a sharable way
+4. Must handle clients with a stale copy of metadata
+
+The famfs kernel component takes care of 1-2 above by caching each file's
+mapping metadata in the kernel.
+
+Requirements 3 and 4 are handled by the user space components, and are
+largely orthogonal to the functionality of the famfs kernel module.
+
+Requirements 3 and 4 cannot be met by conventional fs-dax file systems
+(e.g. xfs) because they use write-back metadata; it is not valid to mount
+such a file system on two hosts from the same in-memory image.
+
+
+Famfs Usage
+===========
+
+Famfs usage is documented at [1].
+
+
+References
+==========
+
+- [1] Famfs user space repository and documentation
+      https://github.com/cxl-micron-reskit/famfs
diff --git a/Documentation/filesystems/index.rst b/Documentation/filesystems/index.rst
index 1f71cf159547..97a9a1f1e96a 100644
--- a/Documentation/filesystems/index.rst
+++ b/Documentation/filesystems/index.rst
@@ -91,6 +91,7 @@ Documentation for filesystem implementations.
    ext3
    ext4/index
    f2fs
+   famfs
    gfs2/index
    hfs
    hfsplus
diff --git a/MAINTAINERS b/MAINTAINERS
index ca7b90a8f0a1..8c282857b103 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9910,6 +9910,7 @@ M:	John Groves <John@Groves.net>
 L:	linux-fsdevel@vger.kernel.org
 L:	linux-cxl@vger.kernel.org
 S:	Supported
+F:	Documentation/filesystems/famfs.rst
 F:	fs/famfs/
 
 FANOTIFY
-- 
2.53.0

Re: [PATCH V12 00/12] famfs: the Fabric-Attached Memory File System (standalone)

[Amir Goldstein]

On Mon, Aug 3, 2026 at 4:27 AM John Groves <john@jagalactic.com> wrote:
>
> From: John Groves <john@groves.net>
>
> This patch series introduces famfs as a standalone file system, as a pivot
> back from fuse/libfuse, for which the porting effort took close to a year, but
> then the review effort started with almost a yearlong delay, and then got
> messy :D [3].
>
> However, after years of maintaining famfs in both forms (standalone and fuse -
> the famfs user space works with both) I think famfs makes more sense as a
> standalone file system - and some influential maintainers have privately
> expressed that opinion to me.

Hi John,

The decision about standalone famfs is left to vfs maintainers.
I am only going to chime in for the fact checks.

Generally speaking, I find your summary of the famfs odyssey very thorough
and mostly accurate, apart from the few bits I will comment on below.

You do bring some valid points that are worth considering, but I would like
the facts to be laid out correctly for the vfs maintainers when approaching to
consider standalone famfs.

>
> The most important thing to know about famfs is that it CANNOT be used as a
> general purpose file system. It is for enabling file-based byte-level access
> (including direct mmap) to very large (e.g. 100TB) shared/disaggregated
> memory appliances - which have become available during this long process,
> and which are in need of Linux support.
>
> So famfs cannot be used by anybody who doesn't know why they need it; Making
> famfs standalone means it can't affect users who don't use it. And my super-
> mega-corp employer (Micron), as well as other memory companies, need it.
> We do not intend to abandon it, but imagine if we did: fs/famfs/ should be
> removed if the memory companies can't be bothered to maintain it.

It does not work this way.
When vfs maintainers accept standalone famfs they accept the burden of
maintaining it whenever vfs internal APIs change even if the filesystem was
abandoned by the mega-corps.
That is the code of the push back for adding new filesystems.
Removing the fs and breaking deployed systems is not an option for the vfs
maintainers.

>
> This code base has been in active CI and used globally by early adopters and
> testers of disaggregated memory. I believe it is solid.
>
> History (skip if you lived it like I did :D)
>
> Famfs was introduced at LPC 2023 and LSFMM 2024 [1] as a standalone file
> system. The fuse odyssey started at LSFMM 24, but I think it's time to
> bring that to a close. The first fuse/libfuse patches came out in April
> 2025 [2] - adding a substantial amount of complexity to the famfs user space,
> and controversial complexity to fuse [3].

This alleged complexity to fuse is attributed to the BPF idea that was shot down
pretty quickly. It is not on the table right now, and representing
fuse maintainers,
I don't think that any of us perceive famfs as adding complexity to fuse.

On the contrary, the sentiment in the review of v11 is that the bits
added to fuse
for famfs are possibly useful for other fuse passthrough use cases.

>
> The fuse community was uncomfortable with the new metadata and fuse message
> formats that famfs needs, going so far as to suggest that famfs use a BPF
> program as a vma fault handler [3] to avoid the need to pass famfs file
> metadata via fuse messages - an idea which, to my relief, was definitively
> shot down at LSFMM 2026 [4].

Again, bringing up the brief BPF design detour is irrelevant to the current
state of affairs and this argument appears over and over in your proposal.

>
> In that session, I was preparing to make the pivot-to-standalone argument,
> and there were others in the room who support that - but Miklos stated that
> he was not opposed to merging famfs as it stands [4], leading me to back off
> of the standalone pivot at the time.
>
> However, 1) that is not how the recent patch discussions have been going [5],
> and 2) I have (not solely on that basis) come to the conclusion that famfs
> makes more sense as a standalone entity. In v11 [5] I dropped the
> controversial famfs interleaved extent (exploding fmaps from well under a
> page to megabytes),

It wasn't an ask from fuse maintainers to cripple famfs.
In fact, Miklos has posted a POC patch [7] for striping, which you
did not respond to at all.

> and there is still a push for adding an intermediate
> virtual backing dev layer that famfs does not need and can't really live
> with. I could go on...

I would really like to hear more about "can't really live with"
because Miklos' patch looks to me like a logic equivalent to standalone
famfs interleaved extent, just a different UAPI, using the abstraction
of virtual backing dev...

>
> My argument
>
> Can we make famfs work in fuse? Yes, if we can agree some things that have
> been quite challenging to agree on. Should we? I think not.
>
> I think famfs adds complexity to fuse that will not likely see constructive
> re-use.

As I wrote, this claim is questionable - it does not match the view of
some of the
fuse maintainers.

> And I think fuse makes famfs worse - significantly more complex, less
> adaptable to change, and less performant. And famfs files are memory - access
> needs to run at memory speeds! In short the risks out-weigh the benefits.

There are definitely pros and cons, but if there was a measurable performance
impact we would not have been having this discussion at all.

AFAIK, the performance impact was with the briefly discussed BPF solution
or worse IOMAP upcalls, so those ideas were dropped immediately.

It is very clear to us that page fault performance is critical to famfs.
I fail to see how Miklos' patch for striping [7] adds significant performance
overhead to page fault and seeing no comment from you on this patch
I am guessing that you never tried to apply it or to measure performance impact.
Am I right?

Look, IMO, your main argument for standalone famfs should be that famfs is
simple and would be easy for vfs maintainers to maintain in the future
in the unlikely
case that it is abandoned.

The way I see it, the main caveat with famfs-fuse is that it takes a toll on
you (a considerable toll) to meet the requirements, but to be fair,
Miklos already
wrote a pretty decent POC and you were offered help from fuse developers
with the remaining parts.

However, the claims about performance overhead and complicating fuse -
I find those inaccurate.

Thanks,
Amir.

>
> About this patch series
>
> I've called it V12 for recent continuity; there was a V1 and a V2 standalone
> in 2024, and then V1-V11 were fuse (call this selective consistency).
>
> Famfs depends on the 'fsdev' dax mode which landed in 7.1 - it will only run
> with an fsdev-mode (aka famfs-mode) daxdev as its backing device(s).
>
> Patch 01 is a fix to the fsdev driver that is needed by the rest of the
> series.
>
> Famfs Overview
>
> Famfs exposes sharable disaggregated memory as a file system. Famfs consumes
> shared memory from [usually shared memory] dax devices, and provides
> memory-mappable files that map directly to the memory - no page cache
> involvement. Famfs differs from conventional file systems in fs-dax mode,
> in that it handles in-memory metadata in a sharable way (which begins with
> never caching dirty shared metadata). So a famfs file system can be mounted
> from multiple nodes, provided they have access to the memory.
>
> The key performance requirement is that famfs must resolve mapping
> faults with minimal overhead. This is achieved by fully caching the
> file-to-devdax metadata for all active files.
>
> Famfs remains the first fs-dax file system that is backed by devdax
> rather than pmem in fs-dax mode (hence the need for the new dax mode).
>
> The famfs user space can be found at [6]
>
> [1] https://lwn.net/Articles/983105/ (Famfs at LSFMM 2024)
> [2] https://lwn.net/Articles/1020170/ (Famfs at LSFMM 2025, with patch link)
> [3] https://lwn.net/Articles/1068686/ (LWN coverage of the patch thread)
> [4] https://lwn.net/Articles/1082687/ (Famfs at LSFMM 2026)
> [5] https://lore.kernel.org/linux-fsdevel/0100019f7d9fbe81-6cb16662-2522-47ea-a152-fab0ee3d9b35-000000@email.amazonses.com/#b
> [6] https://famfs.org
[7] https://lore.kernel.org/fuse-devel/20260601152327.2069787-1-mszeredi@redhat.com/