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

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

[John Groves]

From: John Groves <john@groves.net>

This is v14 of famfs, as a standalone file system. v13 is at [7]; v12 is
at [9].

The intent is that v15 will be fuse again; Miklos and I have come to
agreement on a a path forward with fuse that I think is likely to work.
But this branch has r-b tags, and has some important fixes (which I've
already cross-ported to fuse).

- This series can be had via git as tag famfs-v15 at [10]
- The current draft v15 (fuse) series can be had at tag famfs-v15-rc1 at [11]
- A merge of both can be had as tag famfs-v14-v15-rc1 at [12]

###

Most of this cover letter repeats v12/v13. If you have read it, skip to
"Changes since v13" below and to the per-patch changelogs under the "---"
line of each patch (which carry the v14 notes). The history goes back to 
2023, and is briefly covered in the v12 cover.

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. Micron, as
well as other memory companies, need it, and 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 since 2024. I believe it is solid.

Why standalone (short version)

famfs was introduced at LPC 2023 and LSFMM 2024 [1] as a standalone file
system, then spent close to a year being ported to fuse/libfuse. After
maintaining famfs in both forms, I have concluded it makes more sense as a
standalone file system: fuse adds complexity to famfs (more complex, less
adaptable, less performant - and famfs files are memory, so access must run
at memory speed), while famfs adds complexity to fuse that is unlikely to see
constructive re-use. The full history and argument are in the v12 cover
letter [9].

Changes since v13
-----------------

- Reviewed-by tags from Darrick J. Wong and Jeff Layton have been added to
  the patches they reviewed.

- The MAP_CREATE ioctl (patch 07) is hardened, addressing Richard Cheng's
  review of v13:
    * it now requires CAP_SYS_RAWIO, since MAP_CREATE installs caller-supplied
      device-memory mappings into a file;
    * fmap validation rejects integer overflow (check_add_overflow() on the
      extent arithmetic; file_size and each interleaved extent's ie_nbytes are
      bounded by MAX_LFS_FILESIZE);
    * interleaved (striped) fmap coverage is validated at ingest -- the sum of
      the interleaved extents' ie_nbytes must reach file_size, and each strip
      must physically hold the chunks striped onto it -- so a malformed striped
      file is rejected with -EINVAL rather than faulting (SIGBUS) on access.

- Rebased onto v7.2.

The full v12 -> v13 change list is in the v13 cover [7] and in the per-patch
"---" changelogs.

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).

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).

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] FIXME: v13 lore link
[8] https://lore.kernel.org/linux-cxl/0100019fd478eb93-a6bd48e4-b5f1-4871-93d4-582802956e6c-000000@email.amazonses.com/T/#u (Partial dax patch)
[9] https://lore.kernel.org/linux-fsdevel/20260806053409.GL3560084@frogsfrogsfrogs/T/#t (v12)
[10] https://github.com/jagalactic/linux/tree/famfs-v14
[11] https://github.com/jagalactic/linux/tree/famfs-v15-rc1
[12] https://github.com/jagalactic/linux/tree/famfs-v14-v15-rc1


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                              |   12 +
 fs/famfs/Makefile                             |    5 +
 fs/famfs/famfs_file.c                         | 1070 +++++++++++++++++
 fs/famfs/famfs_inode.c                        |  869 +++++++++++++
 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, 2504 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 V14 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>

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
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

[PATCH V14 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
* module_init uses the MODULE_INCOMPLETE guard to protect bisect-
  ability by preventing registration as a file system until the famfs
  code is complete

Add and export fs/super.c:kill_char_super().

famfs keys its superblock on the backing devdax device's dev_t (via
sget_dev()), so a second mount of the same device shares one super,
much as a block filesystem keys on its block device. As a result
sb->s_dev is a real char-device number: there is no s_bdev, and no
anonymous block device was allocated. None of the existing kill_sb
helpers fit:

  - kill_block_super() releases an s_bdev, which famfs does not have.
  - kill_anon_super()/kill_litter_super() call free_anon_bdev(s_dev),
    but s_dev is the devdax dev_t, not an anon-bdev minor famfs
    allocated; freeing it would corrupt the anonymous-dev IDA.
  - generic_shutdown_super() alone omits kill_super_notify(), which
    unlinks the dying sb from fs_supers and wakes concurrent mounters
    (SB_DEAD); skipping it can leave a dead sb discoverable and hang a
    racing mount.

The correct teardown is generic_shutdown_super() + kill_super_notify()
with no device free. kill_super_notify() is static to fs/super.c, so a
module cannot compose it -- hence this small exported helper.

This commit builds but is otherwise too incomplete to run

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---

 MAINTAINERS                |   7 +
 fs/Kconfig                 |   2 +
 fs/Makefile                |   1 +
 fs/famfs/Kconfig           |  12 ++
 fs/famfs/Makefile          |   5 +
 fs/famfs/famfs_inode.c     | 307 +++++++++++++++++++++++++++++++++++++
 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, 376 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 8014b9f8253e..bfc3acf72c9c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9896,6 +9896,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..c914b6be0627
--- /dev/null
+++ b/fs/famfs/Kconfig
@@ -0,0 +1,12 @@
+
+
+config FAMFS
+       tristate "famfs: shared memory file system"
+       depends on 64BIT
+       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..5a13903da61b
--- /dev/null
+++ b/fs/famfs/famfs_inode.c
@@ -0,0 +1,307 @@
+// 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
+famfs_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 = famfs_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 -ENOPARAM;
+	}
+	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);
+			return -EINVAL;
+		}
+		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 = {
+	.owner		  = THIS_MODULE,
+	.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
+ */
+#define FAMFS_MODULE_INCOMPLETE 1
+
+static int __init
+init_famfs_fs(void)
+{
+	int rc;
+
+	if (FAMFS_MODULE_INCOMPLETE)
+		return -ENODEV;
+
+	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..378544b1aabe
--- /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 famfs_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 ffdcc6a2e0de..bcdfd3563aaa 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1241,6 +1241,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 V14 02/12] famfs: Module operations, fs_context, and mount

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The mount path explicitly rejects concurrent mounts of the same backing device, contradicting the commit message claim that multiple mounts should share the superblock.
--

commit c082254e4802428a0767926c0085c3cbeace41cb
Author: John Groves <john@groves.net>
famfs: Module operations, fs_context, and mount

This commit introduces the initial module operations for famfs, including
filesystem registration, mount parameter parsing, superblock initialization,
and DAX device lookup.

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

[ ... ]

> +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;

[ ... ]

> +	/* 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;

[Severity: Medium]
Does returning -EBUSY here intentionally prevent sharing the superblock
across multiple mounts?

The commit message states: "so a second mount of the same device shares one
super, much as a block filesystem keys on its block device." However, this
check appears to explicitly reject concurrent mounts of the same DAX backing
device rather than sharing the existing superblock.

> +	} else {
> +		pr_debug("%s: initializing new superblock for %s\n",
> +			__func__, fc->source);
> +		famfs_fill_super(sb, fc);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=2

[PATCH V14 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.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_inode.c    | 240 ++++++++++++++++++++++++++++++++++++++
 fs/famfs/famfs_internal.h |  46 ++++++++
 2 files changed, 286 insertions(+)

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index 5a13903da61b..5735d8d1900b 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -76,6 +76,231 @@ 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
  */
@@ -159,6 +384,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) {
@@ -247,6 +484,7 @@ 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;
@@ -258,6 +496,8 @@ 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 378544b1aabe..37f667b2b79a 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 famfs_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 V14 03/12] famfs: Add daxdev table and dax notify_failure support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect memory ordering primitive for publishing lockless state (wmb() vs smp_store_release())
--

commit df7a50134ff93b3213d9bbccb1aca11a71421bee
Author: John Groves <John@Groves.net>

famfs: Add daxdev table and dax notify_failure support

This commit introduces the daxdev table for famfs, which manages multiple dax
devices using a basic structure, primitives, and serialization. It also adds
dax_holder_operations and a notify_failure handler required to properly open
a famfs-mode daxdev.

> diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
> index 5a13903da61b7..5735d8d1900bd 100644
> --- a/fs/famfs/famfs_inode.c
> +++ b/fs/famfs/famfs_inode.c

[ ... ]

> @@ -76,6 +76,231 @@ famfs_get_inode(
> +int
> +famfs_install_daxdev(
> +	struct famfs_fs_info *fsi,
> +	struct super_block *sb,
> +	u64 index,
> +	dev_t devno,
> +	const char *name)
> +{

[ ... ]

> +		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;

[Severity: High]
Does this code safely publish the daxdev structure to concurrent lockless
readers?

In famfs_install_daxdev(), the structure is populated and then its valid flag
is set to 1 after a naked wmb(). While wmb() provides a write barrier for the
writer, it does not enforce pairing on the reader side. If a concurrent
lockless reader (such as famfs_daxdev_from_index() added later in this patch
series) checks valid using a simple read like if (!dd->valid) without a
corresponding read barrier, the CPU could reorder the reads.

If the reads are reordered, the reader could observe valid == 1 but see
uninitialized data or a NULL devp, which might lead to a NULL pointer
dereference.

Could this be converted to use smp_store_release(&daxdev->valid, 1) on the
writer side, and paired with smp_load_acquire() on the lockless reader side?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=3

[PATCH V14 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

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---

 fs/famfs/famfs_inode.c    | 289 +++++++++++++++++++++++++++++++++++++-
 fs/famfs/famfs_internal.h |   6 +
 2 files changed, 292 insertions(+), 3 deletions(-)

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index 5735d8d1900b..b7a3d8d6ee8c 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,
@@ -55,11 +58,11 @@ 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 ".") */
@@ -73,6 +76,286 @@ 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 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);
+	/* Adding an entry updates the parent directory's mtime/ctime */
+	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
+
+	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;
+	struct inode *target = d_inode(new_dentry);
+
+	if (!famfs_opt_enabled(fsi, FAMFS_OPT_RENAME))
+		return -EPERM;
+
+	/*
+	 * Renaming over an existing target removes it, so require the same
+	 * policy gate that unlink()/rmdir() would. RENAME_EXCHANGE swaps the
+	 * two entries rather than removing the target, so it is exempt.
+	 */
+	if (target && !(flags & RENAME_EXCHANGE)) {
+		if (S_ISDIR(target->i_mode)) {
+			if (!famfs_opt_enabled(fsi, FAMFS_OPT_RMDIR))
+				return -EPERM;
+		} else if (target->i_private &&
+			   !famfs_opt_enabled(fsi, FAMFS_OPT_UNLINK)) {
+			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)
  */
@@ -312,7 +595,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 37f667b2b79a..30f0b01010d5 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 famfs_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

[PATCH V14 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.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---

 fs/famfs/Makefile         |   2 +-
 fs/famfs/famfs_file.c     | 141 ++++++++++++++++++++++++++++++++++++++
 fs/famfs/famfs_inode.c    |   2 +-
 fs/famfs/famfs_internal.h |   2 +
 4 files changed, 145 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..1369fe1824bc
--- /dev/null
+++ b/fs/famfs/famfs_file.c
@@ -0,0 +1,141 @@
+// 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);
+
+	if (rc > 0)
+		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;
+	}
+
+	kiocb_modified(iocb); /* mtime/ctime + strip set[e]uid */
+
+	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	   = copy_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 b7a3d8d6ee8c..9e8662c4ac98 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -59,7 +59,7 @@ 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 30f0b01010d5..ff9f1d3f686e 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 V14 05/12] famfs: Introduce file_operations read/write

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Ignored return value of kiocb_modified() in famfs_dax_write_iter bypasses security state updates.
- [High] Bypass of generic_write_checks() breaks O_APPEND semantics and incorrectly returns 0 on EOF, causing infinite write loops.
- [High] 64-bit file size is truncated to 32-bit size_t, breaking I/O operations on files larger than 4GB on 32-bit platforms.
- [Medium] famfs_dax_read_iter() and famfs_dax_write_iter() ignore IOCB_NOWAIT and use blocking lock acquisitions.
- [Medium] Kernel pointer leak via pr_debug by intentionally bypassing pointer hashing.
--

commit 31958b8d418a1061bd63bd9bed3ba0558707e229
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.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> new file mode 100644
> index 0000000000000..1369fe1824bc8
> --- /dev/null
> +++ b/fs/famfs/famfs_file.c
> @@ -0,0 +1,141 @@

[ ... ]

> +/* 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: Medium]
Does this cast to u64 and %llx format specifier bypass the kernel's pointer
hashing mechanism, potentially leaking raw kernel memory addresses?

> +		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 truncating the 64-bit loff_t returned by i_size_read() into a 32-bit
size_t variable break I/O operations for files larger than 4GB on 32-bit
platforms?

> +	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;

[Severity: High]
When a write hits the end of the file, this logic returns 0 instead of a
proper error code like -ENOSPC or -EFBIG.  Can this cause infinite write
loops in standard userspace tools?

> +
> +	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);

[Severity: Medium]
Does this unconditional blocking lock acquisition ignore the IOCB_NOWAIT
flag, potentially stalling asynchronous I/O frameworks like io_uring?

> +	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);
> +
> +	if (rc > 0)
> +		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);

[Severity: Medium]
Similar to famfs_dax_read_iter(), does this unconditional blocking lock
acquisition ignore the IOCB_NOWAIT flag, potentially stalling asynchronous
I/O frameworks?

> +	rc = famfs_rw_prep(iocb, from);

[Severity: High]
Does the bypass of generic_write_checks() here break O_APPEND semantics?
It appears iocb->ki_pos is not updated to EOF, causing the write to
overwrite existing data instead of appending.

> +	if (rc || !iov_iter_count(from)) {
> +		inode_unlock(inode);
> +		return rc;
> +	}
> +
> +	kiocb_modified(iocb); /* mtime/ctime + strip set[e]uid */

[Severity: High]
Does ignoring the return value of kiocb_modified() bypass error checking?
If stripping the setuid/setgid bit fails, could this allow a malicious
user to retain privileges on a file while modifying its contents?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=5

[PATCH V14 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.

Note that famfs supports MAP_SYNC (for MMAP_SYNC) basically for free,
because file maps are never mutated after MAP_CREATE.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---

 fs/famfs/famfs_file.c | 109 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index 1369fe1824bc..9c1cd2f67489 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -16,6 +16,76 @@
 
 #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,
+	.page_mkwrite	= famfs_filemap_mkwrite,
+	.pfn_mkwrite	= famfs_filemap_mkwrite,
+};
+
 /*********************************************************************
  * file_operations
  */
@@ -120,14 +190,51 @@ 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. Reject a mapping
+	 * that is already writable, and strip VM_MAYWRITE from a read-only
+	 * shared mapping so a later mprotect(PROT_WRITE) cannot upgrade it.
+	 * This is best effort: it does not revoke mappings that already exist.
+	 */
+	if ((vma->vm_flags & VM_SHARED) &&
+	    !famfs_opt_enabled(fsi, FAMFS_OPT_WRITE)) {
+		if (vma->vm_flags & VM_WRITE)
+			return -EPERM;
+		vm_flags_clear(vma, VM_MAYWRITE);
+	}
+
+	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,
 
+	/* fmap metadata is immutable after MAP_CREATE, so MAP_SYNC is free */
+	.fop_flags	   = FOP_MMAP_SYNC,
+
 	/* 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 */,
+	.mmap		   = famfs_file_mmap,
 
 	/* Force PMD alignment for mmap */
 	.get_unmapped_area = thp_get_unmapped_area,
-- 
2.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Modifying VMA flags in a legacy .mmap callback breaks VMA merging, leading to memory fragmentation.
--

commit f0dd25d565afad641be49c7b1903722fa578648a
Author: John Groves <john@groves.net>

    famfs: Introduce mmap and VM fault handling

    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.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index 1369fe1824bc8..9c1cd2f674893 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
[ ... ]
> @@ -120,14 +190,51 @@ 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. Reject a mapping
> +	 * that is already writable, and strip VM_MAYWRITE from a read-only
> +	 * shared mapping so a later mprotect(PROT_WRITE) cannot upgrade it.
> +	 * This is best effort: it does not revoke mappings that already exist.
> +	 */
> +	if ((vma->vm_flags & VM_SHARED) &&
> +	    !famfs_opt_enabled(fsi, FAMFS_OPT_WRITE)) {
> +		if (vma->vm_flags & VM_WRITE)
> +			return -EPERM;
> +		vm_flags_clear(vma, VM_MAYWRITE);

[Severity: Medium]
Does clearing VM_MAYWRITE in the legacy f_op->mmap callback break VMA
merging?

Since this occurs post-merge attempt, these modified flags diverge from the
initially proposed flags. Subsequent adjacent mmaps of the same file with
identical protection will fail to merge.

Should this driver implement f_op->mmap_prepare() instead to modify VMA
flags before the merge attempt?

> +	}
> +
> +	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);

[Severity: Medium]
Will setting VM_HUGEPAGE here guarantee that adjacent default mappings fail
to merge?

Like VM_MAYWRITE, modifying this flag post-merge attempt prevents the core MM
subsystem from successfully merging new mappings with existing ones.

Should VM_HUGEPAGE also be set via f_op->mmap_prepare() instead?

> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=6

[PATCH V14 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.

Famfs now accepts extent and chunk alignment at either PMD or PTE. PMD
is the normal case, and it guarantees PMD faults - which is right for
large files, which are the original use case. But there are also active
use cases that use zillions of small files, and those need PTE support
to avoid big space amplification.

Also add famfs ioctls to ioctl-number.rst

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---
v14:
 - Require CAP_SYS_RAWIO in the MAP_CREATE handler (famfs_file_init_dax()).
   MAP_CREATE installs caller-supplied device-memory mappings into a file,
   so it now demands CAP_SYS_RAWIO in addition to the mount-wide
   FAMFS_OPT_MAP_CREATE policy gate (Richard Cheng).
 - Reject integer overflow while validating a caller-supplied fmap. Each
   extent's ext_offset + ext_len and the running extent_total are now
   accumulated with check_add_overflow(), and file_size (and each
   interleaved extent's ie_nbytes) is bounded by MAX_LFS_FILESIZE. Without
   this an overflowing extent could wrap a dax offset back toward 0 and
   resolve to the wrong memory (Richard Cheng; Darrick Wong).
 - Validate interleaved (striped) fmap coverage at ingest, so a malformed
   striped file is rejected with -EINVAL at MAP_CREATE rather than faulting
   (SIGBUS) on access. Two checks: (a) the sum of the interleaved extents'
   ie_nbytes must reach file_size (logical coverage); and (b) each strip
   must physically hold the chunks striped onto it -- checked by probing
   the highest file offset that lands on each strip (the last byte of each
   top-row chunk) and resolving it with the same arithmetic the fault path
   uses, which is partial-final-chunk aware and assumes nothing about
   relative strip sizes. Previously the check subtracted physical strip
   ext_len from file_size, conflating physical allocation with logical
   coverage (Richard Cheng). This is also the MAP_CREATE-time interleaved
   validation discussed with Darrick Wong on the opts patch.

 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 fs/famfs/famfs_file.c                         | 387 +++++++++++++++++-
 fs/famfs/famfs_inode.c                        |   1 +
 fs/famfs/famfs_internal.h                     |  46 +++
 include/uapi/linux/famfs_ioctl.h              |  91 ++++
 5 files changed, 525 insertions(+), 1 deletion(-)
 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 9c1cd2f67489..f099e0d08673 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -13,9 +13,377 @@
 #include <linux/mm.h>
 #include <linux/dax.h>
 #include <linux/iomap.h>
+#include <linux/capability.h>
+#include <linux/overflow.h>
 
+#include <linux/famfs_ioctl.h>
 #include "famfs_internal.h"
 
+/* Expose famfs kernel abi version as a read-only module parameter */
+static int kabi_version = FAMFS_KABI_VERSION;
+module_param(kabi_version, int, 0444);
+MODULE_PARM_DESC(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);
+}
+
+static int
+famfs_check_ext_alignment(struct famfs_meta_simple_ext *se)
+{
+	int errs = 0;
+
+	if (!IS_ALIGNED(se->ext_offset, PAGE_SIZE))
+		errs++;
+	if (!IS_ALIGNED(se->ext_len, PAGE_SIZE))
+		errs++;
+
+	return errs;
+}
+
+/**
+ * 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_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;
+	u64 ext_end;
+
+	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;
+	/* MAP_CREATE maps caller-supplied device memory; require CAP_SYS_RAWIO */
+	if (!capable(CAP_SYS_RAWIO))
+		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;
+	if (fmh.file_size > MAX_LFS_FILESIZE)
+		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;
+			}
+			if (check_add_overflow(meta->se[i].ext_offset,
+					       meta->se[i].ext_len, &ext_end) ||
+			    check_add_overflow(extent_total,
+					       meta->se[i].ext_len,
+					       &extent_total)) {
+				rc = -EINVAL;
+				goto out;
+			}
+			meta->dev_bitmap |= BIT_ULL(meta->se[i].dev_index);
+			errs += famfs_check_ext_alignment(&meta->se[i]);
+		}
+		break;
+	}
+
+	case FAMFS_IOC_EXT_INTERLEAVE: {
+		u64 logical_total = 0;
+		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;
+			u64 chunks;
+			u64 n, nprobe;
+
+			next_offset += sizeof(*ie_in);
+			if (next_offset > fmh.fmap_size) {
+				rc = -EINVAL;
+				goto out;
+			}
+
+			/* chunk_size must be exactly one supported alloc unit */
+			if (ie_in->ie_chunk_size != PAGE_SIZE &&
+			    ie_in->ie_chunk_size != PMD_SIZE) {
+				rc = -EINVAL;
+				goto out;
+			}
+			if (ie_in->ie_nbytes == 0 ||
+			    ie_in->ie_nbytes > MAX_LFS_FILESIZE) {
+				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;
+
+			chunks = DIV_ROUND_UP(ie_in->ie_nbytes,
+					      ie_in->ie_chunk_size);
+
+			/* 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;
+				}
+				if (check_add_overflow(so->ext_offset,
+						       so->ext_len, &ext_end) ||
+				    check_add_overflow(extent_total,
+						       so->ext_len,
+						       &extent_total)) {
+					rc = -EINVAL;
+					goto out;
+				}
+				meta->dev_bitmap |= BIT_ULL(so->dev_index);
+				errs += famfs_check_ext_alignment(so);
+			}
+
+			/*
+			 * Validate that every strip physically holds the chunks
+			 * striped onto it. Rather than trust the strip sizes,
+			 * probe the highest file offset that lands on each strip
+			 * -- the last byte of each last-stripe chunk -- and
+			 * resolve it with the same arithmetic the fault path
+			 * uses. If those worst-case offsets resolve within their
+			 * strips, every lower offset does too. Strips beyond the
+			 * chunk count map nothing and are skipped.
+			 */
+			nprobe = min_t(u64, nstrips, chunks);
+			for (n = 0; n < nprobe; n++) {
+				u64 c          = chunks - 1 - n;
+				u64 stripe_num = c / nstrips;
+				u64 strip_num  = c % nstrips;
+				u64 chunk_last = (c == chunks - 1) ?
+					((ie_in->ie_nbytes - 1) %
+					 ie_in->ie_chunk_size) :
+					(ie_in->ie_chunk_size - 1);
+				u64 strip_off  = stripe_num *
+					ie_in->ie_chunk_size + chunk_last;
+
+				if (strip_off >=
+				    meta->ie[i].ie_strips[strip_num].ext_len) {
+					rc = -EINVAL;
+					goto out;
+				}
+			}
+
+			/* This interleaved extent maps ie_nbytes logical bytes */
+			if (check_add_overflow(logical_total, ie_in->ie_nbytes,
+					       &logical_total)) {
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+
+		if (logical_total < meta->file_size) {
+			/* Interleaved extents 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
  */
@@ -94,6 +462,23 @@ 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);
@@ -233,7 +618,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 9e8662c4ac98..98c877a6009e 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -341,6 +341,7 @@ 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 ff9f1d3f686e..f17380d24f6d 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 V14 07/12] famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing memory barriers when publishing and reading `inode->i_private` allows concurrent readers to see uninitialized metadata.
- [Medium] Reserved fields in the UAPI structs are not validated to be zero, breaking forward compatibility.
--

commit 01eb70d8c2de27c662f746e15fb2d3e809666490
Author: John Groves <John@Groves.net>

famfs: MAP_CREATE ioctl and fmap ingest (ABI 44)
    
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.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index 9c1cd2f674893..f099e0d08673f 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
[ ... ]
> @@ -13,9 +13,377 @@
[ ... ]
> +static int
> +famfs_file_init_dax(struct file *file, void __user *arg)
> +{
> +	struct famfs_ioc_fmap_header fmh;
[ ... ]
> +	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;

[Severity: Medium]
Are the reserved and reserved1 fields in famfs_ioc_fmap_header validated
to be zero here? 

If userspace passes uninitialized or garbage data in these fields, the kernel
will blindly accept it. This could prevent the kernel from safely assigning
meaning to these fields in the future without breaking backward compatibility,
since it won't be able to distinguish between older applications passing
garbage and newer applications intentionally using the flags. 

The reserved fields in famfs_ioc_simple_ext and famfs_ioc_iext also appear
to lack zero-validation.

> +	if (fmh.fmap_size > FAMFS_FMAP_MSG_MAX)
> +		return -EFBIG;
[ ... ]
> +	/* 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]
Does this assignment to inode->i_private need a release memory barrier?

This plain store publishes the structure without a release barrier, allowing
prior initialization stores to be reordered after it. A concurrent reader
could observe a non-NULL inode->i_private pointer but read uninitialized or
stale metadata fields.

For instance, a concurrent mmap() on the same file calls famfs_file_mmap()
which in turn calls famfs_file_invalid(inode). Because famfs_file_mmap()
does not hold inode_lock, famfs_file_invalid() will execute a plain load
(struct famfs_file_meta *meta = inode->i_private;) without an acquire
barrier. 

If it reads uninitialized data like meta->file_size, it could fail the 
i_size != meta->file_size check, which permanently sets meta->error = true,
thereby permanently bricking the file mapping.

Should smp_store_release() and smp_load_acquire() be used here to enforce
the required memory ordering?

> +		inode->i_flags |= S_DAX;
> +		meta = NULL; /* owned by the inode now */
> +		rc = 0;
> +	}
> +	inode_unlock(inode);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=7

[PATCH V14 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.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---

 fs/famfs/famfs_file.c | 305 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 302 insertions(+), 3 deletions(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index f099e0d08673..2200f168aa79 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -384,6 +384,305 @@ 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,
+	loff_t len)
+{
+	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;
+
+	/* 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;
+	}
+
+	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;
+
+			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;
+
+	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)
+ *
+ * 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,
+	loff_t len)
+{
+	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);
+
+	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;
+
+			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;
+
+	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);
+}
+
+/* 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
  */
@@ -412,7 +711,7 @@ __famfs_filemap_fault(
 		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);
 
@@ -534,7 +833,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);
 
 	if (rc > 0)
@@ -570,7 +869,7 @@ famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	kiocb_modified(iocb); /* mtime/ctime + strip set[e]uid */
 
-	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 V14 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:
- [High] Missing read memory barrier in lockless daxdev lookup causes NULL pointer dereferences and uninitialized iomap returns.
- [High] Huge user-provided extent lengths can bypass overflow checks, wrap to negative `loff_t`, and cause `iomap_begin` to return a negative length.
- [High] Concurrent lockless modification of latching error state (meta->error) from page fault paths.
- [High] Page fault path can observe partially initialized inode state due to missing synchronization.
- [Medium] Concurrent read of `dd->name` in `famfs_dax_err` while `dd->valid == 0` can dereference uncommitted or unmapped memory.
- [Medium] Un-ratelimited pr_warn in the I/O path can cause kernel log flooding.
- [Low] Native 64-bit division and modulo operations cause build failures on 32-bit architectures.

Pre-existing issues:
- [High] famfs_dax_read_iter manually advances iocb->ki_pos after dax_iomap_rw already advanced it.
--

commit b15edec08ab799f362ddce177190dc4582c58868
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.
Wires it into the read, write and fault paths by replacing their NULL
stub with &famfs_iomap_ops, so dax_iomap_rw() and dax_iomap_fault()
now resolve through famfs.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index f099e0d08673f..2200f168aa79d 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
> @@ -384,6 +384,305 @@ famfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
[ ... ]
> +static int
> +famfs_dax_err(struct famfs_daxdev *dd)
> +{
> +	if (!dd->valid) {
> +		pr_debug("%s: daxdev=%s invalid\n", __func__, dd->name);

[Severity: Medium]
Can this race with daxdev initialization? A concurrent
famfs_install_daxdev() allocates dd->name via kstrdup() before setting
dd->valid = 1. Could a reader here observe the pointer but dereference
uncommitted string memory?

> +		return -EIO;
> +	}
[ ... ]
> +static struct dax_device *
> +famfs_daxdev_from_index(struct famfs_fs_info *fsi, u64 dev_index, int *errp)
> +{
[ ... ]
> +	dd = &devlist->devlist[dev_index];
> +	rc = famfs_dax_err(dd);
> +	if (rc) {
> +		*errp = rc;
> +		return NULL;
> +	}
> +	*errp = 0;
> +	return dd->devp;

[Severity: High]
Does this lockless read of dd->devp need an smp_rmb()?

The writer famfs_install_daxdev() uses a wmb() to order stores before
setting dd->valid = 1. If the reader reads dd->valid == 1 in
famfs_dax_err(), could CPU reordering cause it to fetch a stale NULL
for dd->devp, returning NULL while setting *errp = 0?

> +}
> +
> +static int
> +famfs_meta_to_dax_offset_interleaved(
[ ... ]
> +		/* 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]
Will these native 64-bit division and modulo operations cause build
failures on 32-bit architectures? Should this use kernel math helpers
like div64_u64()?

> +			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;
[ ... ]
> +			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]
If a user provides a very large strip->ext_len that exceeds the maximum
signed loff_t (e.g. > 0x7FFFFFFFFFFFFFFF), will casting it to loff_t in
min_t() result in a negative number? Could this cause iomap->length to
become negative and break VFS iomap iterator invariants?

> +			iomap->dax_dev = daxdev;
> +			iomap->type    = IOMAP_MAPPED;
> +
> +			return 0;
[ ... ]
> +static int
> +famfs_meta_to_dax_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__);

[Severity: Medium]
Since this is on the hot I/O path for every upcall, could an
unprivileged user trivially flood the kernel log by reading a mapped
file with ext_offset set to 0? Should this use a ratelimited printk?

> +
> +		/* 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;

[Severity: High]
If reading a file races with famfs_daxdev_open(), could failing to find
the daxdev prematurely latch meta->error = true? Since this flag is
persistent, does this permanently deny access to the file? Also, is it
safe to write to meta->error locklessly from concurrent fault paths?

Additionally, if famfs_daxdev_from_index() returns NULL with rc = 0 due
to reordering (as asked earlier), does this return 0 to the VFS while
leaving iomap->length uninitialized, causing iterators to malfunction?

> +				return rc;
> +			}
[ ... ]
> +			iomap->addr    = dax_ext_offset + local_offset;
> +			iomap->offset  = file_offset;
> +			iomap->length  = min_t(loff_t, len, ext_len_remainder);

[Severity: High]
Similar to the interleaved case, if dax_ext_len is extremely large,
could it cast to a negative loff_t, resulting in a negative
ext_len_remainder and causing iomap->length to become negative?

> +			iomap->dax_dev = daxdev;
> +			iomap->type    = IOMAP_MAPPED;
> +
> +			return 0;
> +		}
[ ... ]
> @@ -412,7 +711,7 @@ __famfs_filemap_fault(
>  		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);

[Severity: High]
Looking at __famfs_filemap_fault(), it locklessly checks IS_DAX(inode).
Since famfs_file_init_dax() sets inode->i_private, updates i_size, and
sets S_DAX without memory barriers:

famfs_file_init_dax() {
    inode->i_private = meta;
    i_size_write(inode, meta->file_size);
    inode->i_flags |= S_DAX;
}

Could the page fault path observe IS_DAX being true, but read a stale
i_size of 0? If so, would famfs_file_invalid() incorrectly latch
meta->error = true because i_size != meta->file_size?

>  	if (ret & VM_FAULT_NEEDDSYNC)
>  		ret = dax_finish_sync_fault(vmf, order, pfn);
>  
> @@ -534,7 +833,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);
>  
>  	if (rc > 0)

[Severity: High]
This isn't a bug introduced by this patch, but by replacing the NULL ops
with &famfs_iomap_ops, does dax_iomap_rw() now succeed and inherently
update iocb->ki_pos? If so, does the subsequent manual advance of
iocb->ki_pos via iocb->ki_pos += rc (just below this diff) cause a
double-advance of the file offset during sequential reads?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=8

[PATCH V14 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.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
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 2200f168aa79..eba80dab0562 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -351,6 +351,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 famfs_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;
+	int rc;
+
+	char *path __free(kfree) = NULL;
+
+	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(no_free_ptr(path));
+
+	rc = famfs_lookup_daxdev(path, &devno);
+	if (rc)
+		return rc;
+
+	/*
+	 * 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);
+	return rc;
+}
+
 /**
  * famfs_file_ioctl() - Top-level famfs file ioctl handler
  * @file: the file
@@ -372,6 +442,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 V14 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:
- [High] Lockless read of daxdev->valid lacks a read memory barrier, leading to a TOCTOU race that can yield a NULL pointer and a zero error code.
--

commit 592a1b20aa272f01a78424966dd5d63af6c63cf8
Author: John Groves <John@Groves.net>

famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN)

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.

> kernel via FAMFSIOC_DAXDEV_OPEN). This adds daxdevs to daxdev_table for

This isn't a bug, but there is an unmatched closing parenthesis after
FAMFSIOC_DAXDEV_OPEN.

> diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> index 2200f168aa79d..eba80dab0562d 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
[ ... ]
> +static int
> +famfs_daxdev_open(struct file *file, void __user *arg)
> +{
[ ... ]
> +	/*
> +	 * 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);

[Severity: High]
Does exposing famfs_install_daxdev() to concurrent execution via this ioctl
trigger a race condition in the lockless readers?

famfs_install_daxdev() uses wmb() to ensure daxdev->devp is visible before
setting daxdev->valid = 1:

fs/famfs/famfs_inode.c:famfs_install_daxdev() {
    ...
    wmb(); /* All other fields must be visible before valid */
    daxdev->valid = 1;
    ...
}

However, the readers famfs_dax_err() and famfs_daxdev_from_index() lack a
pairing smp_rmb():

fs/famfs/famfs_file.c:famfs_dax_err() {
    if (!dd->valid) {
        pr_debug("%s: daxdev=%s invalid\n", __func__, dd->name);
        return -EIO;
    }
    ...
}

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

Could the CPU or compiler reorder the loads, reading a stale NULL for devp
before reading valid == 1, resulting in famfs_daxdev_from_index() returning
NULL with a success error code (0)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=9

[PATCH V14 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.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_file.c            | 59 ++++++++++++++++++++++++++++++++
 fs/famfs/famfs_inode.c           |  1 +
 fs/famfs/famfs_internal.h        | 30 +++++++++++++---
 include/uapi/linux/famfs_ioctl.h | 45 ++++++++++++++++++++++++
 4 files changed, 131 insertions(+), 4 deletions(-)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index eba80dab0562..0a6d0392794c 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -421,6 +421,53 @@ 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
@@ -442,6 +489,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 98c877a6009e..2f1938adbfbc 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -769,6 +769,7 @@ 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 f17380d24f6d..5e043b5a0ddd 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 famfs_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

[PATCH V14 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)

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: John Groves <john@groves.net>
---
 drivers/dax/fsdev.c       | 19 +++++++++++++++
 fs/famfs/famfs_file.c     |  5 ++++
 fs/famfs/famfs_inode.c    | 49 ++++++++++++++++++++++++++++++++++-----
 fs/famfs/famfs_internal.h |  8 +++++++
 include/linux/dax.h       |  1 +
 5 files changed, 76 insertions(+), 6 deletions(-)

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 0a6d0392794c..965b6732f52c 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -344,6 +344,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 2f1938adbfbc..61b668b299ef 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"
 
@@ -348,8 +349,38 @@ 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,
@@ -444,6 +475,7 @@ famfs_install_daxdev(
 	const char *name)
 {
 	struct famfs_daxdev *daxdev;
+	struct dax_device *devp = NULL;
 	int rc = 0;
 
 	if (index >= fsi->dax_devlist->nslots) {
@@ -507,6 +539,15 @@ 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;
@@ -769,6 +810,7 @@ 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;
@@ -802,16 +844,11 @@ static struct file_system_type famfs_fs_type = {
 /******************************************************************************
  * Module stuff
  */
-#define FAMFS_MODULE_INCOMPLETE 1
-
 static int __init
 init_famfs_fs(void)
 {
 	int rc;
 
-	if (FAMFS_MODULE_INCOMPLETE)
-		return -ENODEV;
-
 	rc = register_filesystem(&famfs_fs_type);
 
 	return rc;
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index 5e043b5a0ddd..8765be41e4a7 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 V14 11/12] famfs: Report device capacity via statfs so df works

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Use-after-free race condition in `dax_fsdev_size()` due to missing SRCU read lock (`dax_read_lock()`).
- [Medium] The statfs `used_capacity` counter grows monotonically but is never decremented when files are evicted, causing `df` to permanently show 0 available space.
- [Low] Mixing goto-based error handling with scope-based cleanup helpers (`scoped_guard`) in the same function violates kernel cleanup guidelines.
--

commit dd30d0d60f5f3c8708d001e5e982ce4519316fdc
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.

> 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 this code introduce a use-after-free race condition?

dax_fsdev_size() retrieves the dev_dax pointer via dax_get_private(dax_dev)
and accesses dev_dax->cached_size without acquiring the SRCU read lock
(dax_read_lock()).

If an asynchronous device unplug or sysfs unbind occurs concurrently,
kill_dax() could clear the DAXDEV_ALIVE flag and call synchronize_srcu().
Since the SRCU read lock is not held here, synchronize_srcu() would return
immediately, and the unplug path could free the dev_dax structure before
cached_size is read.

> +}
> +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 0a6d0392794c5..965b6732f52c4 100644
> --- a/fs/famfs/famfs_file.c
> +++ b/fs/famfs/famfs_file.c
[ ... ]
> @@ -344,6 +344,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: Low]
This isn't a bug, but does mixing scope-based cleanup helpers (scoped_guard)
with goto-based error handling in this function violate kernel cleanup
guidelines?

famfs_file_init_dax() heavily relies on goto labels (out:) for error
handling. The kernel guidelines suggest either converting all resources
that need cleanup to scope-based cleanup, or converting none of them, to
avoid confusing ownership semantics.

>  out:
>  	kvfree(fmap_buf);
>  	if (meta)
[ ... ]
> diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
> index 2f1938adbfbc3..61b668b299ef5 100644
> --- a/fs/famfs/famfs_inode.c
> +++ b/fs/famfs/famfs_inode.c
[ ... ]
> @@ -348,8 +349,38 @@ famfs_evict_inode(struct inode *inode)
>  	clear_inode(inode);
>  }
        ^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will this cause the fsi->used_capacity counter to grow monotonically and
eventually report zero available space?

When a user creates and maps a famfs file, fsi->used_capacity is permanently
incremented in famfs_file_init_dax(). However, famfs_evict_inode() frees the
metadata but does not appear to decrement fsi->used_capacity to reflect the
freed space.

If users repeatedly create and unlink files, won't fsi->used_capacity
double-count the space until df permanently shows 0 available blocks?

>  
> +/*
> + * 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
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000000@email.amazonses.com?part=11

[PATCH V14 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>

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
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 bfc3acf72c9c..515a3d832397 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9901,6 +9901,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