mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems
@ 2026-08-01 22:55 Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 01/10] ufs: name the modern UFS2 superblock fields Ali Ahmet Memis
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

The recent fs/ufs traffic has mostly been hardening against crafted
images. This is the other kind: filesystems that are entirely valid,
written by FreeBSD or by Linux itself, that fs/ufs then handles wrongly.
Most of it comes from comparing the code against what modern FreeBSD
writes; Linux last looked at the UFS2 layout around FreeBSD 5.x, and
several fields it still treats as spare have been in use for years.

Patch 1 is preparation with no functional change: it splits
fs_44.fs_sparecon[50] into the fields FreeBSD defines. The layout does
not move. sizeof(struct ufs_super_block_third) stays 356 and
fs_contigsumsize stays at offset 292, checked with the compiler before
and after, and cross-checked against a superblock written by mkfs.ufs,
where the same field sits at absolute offset 1316 with usb3 based at
1024.

Patches 2 and 3 are extended attribute fallout. A UFS2 inode can own
attribute blocks in di_extb[], which ffs_alloc() counts in di_blocks, so
deciding fast symlinks by i_blocks misfires on a short symlink that has
an attribute, and unlinking any inode with attributes leaks the blocks.

Patches 4 and 5 are the on-disk file flags. New inodes inherit the
parent directory's chflags, which FreeBSD never does, and the flags that
are there never reach S_IMMUTABLE or S_APPEND.

Patch 6 is an arithmetic bug in ufs_change_blocknr(), where a buffer
index within the folio is used as an offset into the relocated range.
It needs a page larger than the filesystem block. It does not touch the
deeper relocation problems described in [1], only the offset.

Patches 7 to 10 are mount policy. The read-only decision taken at mount
time can be undone with remount,rw, and nothing looks at the modern
feature flags, so filesystems needing fsck or journal replay, or holding
snapshots or metadata check hashes, are written to as if ordinary.

Testing
-------

Built with W=1 and checkpatch --strict clean on all ten. Runtime tested
in qemu against images from mkfs.ufs (Debian ufsutils 8.2). Cases that
need on-disk state Linux cannot create were produced by editing the
images directly. Every case was run twice, once on this series and once
on the unpatched base, same images, same script.

x86_64, KASAN, UFS2 16384/2048 and UFS1 8192/1024:

  case                                    unpatched          patched
  readlink, short symlink with extattrs   I/O beyond end     correct target
  unlink of the same symlink              ufs_panic          clean
  extattr fragment after inode delete     still allocated    free
  new inode under a flagged directory     flags 0x1          flags 0
  create inside an SF_IMMUTABLE dir       succeeds           EPERM
  remount,rw with fs_clean FSACTIVE       succeeds           refused
  FS_NEEDSFSCK / FS_SUJ read-write        read-write         read-only
  active snapshot read-write              read-write         read-only
  FS_METACKHASH read-write                read-write         read-only
  FS_METACKHASH with no hashes set        read-write         read-write

The unpatched readlink failure is the chain patch 2 describes, with the
ASCII of the link target used as a block pointer:

  vdl: rw=8388608, sector=15116822126868137148, nr_sectors = 4 limit=131072
  ufs: panic: ufs_free_fragments: freeing blocks are outside device

The last line of the table is a negative control: a filesystem that
advertises check hashes without maintaining any stays writable, so patch
10 is not simply refusing everything. 26 of 26 checks pass on the
series, KASAN quiet on both runs.

arm64 with 64 KiB pages, UFS1 4096/512, for patch 6:

  case                                    unpatched          patched
  file survives a tail relocation         corrupt at 4096    intact
  neighbouring file after relocation      intact             intact

A block is 8 fragments there and blks_per_page is 128, so a tail in the
second block gives beg 8 and beg & mask 8. With the fragments that
physically follow the tail occupied, extending the file relocates it and
the first byte of the moved tail reads back as zero. The same test on 4
KiB pages never reaches a nonzero beg & mask, which is why the bug is
invisible there.

Patch 10 is the one I am least sure belongs. Refusing read-write on
check-hashed filesystems is correct, but FreeBSD enables check hashes by
default, so images that mount read-write today would stop. That is a
policy call rather than a bug fix, and the patch is independent; drop it
and the other nine still apply.

Not addressed: UFS still has no extended attribute or ACL support, so
FS_ACLS filesystems are read with their access control ignored, and
fs_clean is never set to FSACTIVE while mounted read-write, so a crash
leaves a filesystem looking clean. Both are larger jobs.

Two things I looked at and dropped rather than send: the 4 GiB directory
truncation in ufs_last_byte() is already posted by YANXIN LI [2], and
the UFS_FSLOG read-write behaviour turned out to be deliberate, added in
2010 by Alex Viskovatoff with Evgeniy Dushistov's ack [3].

[1] https://lore.kernel.org/all/20241018231428.GC1172273@ZenIV/
[2] https://lore.kernel.org/all/20260718190300.867109-1-fadouse@pm.me/
[3] https://lore.kernel.org/all/1265683224.4096.1358983441@webmail.messagingengine.com/

Ali Ahmet Memis (10):
  ufs: name the modern UFS2 superblock fields
  ufs: use i_size to detect fast symlinks
  ufs: free UFS2 external attribute blocks on inode deletion
  ufs: do not inherit file flags from the parent directory
  ufs: honour on-disk immutable and append-only flags
  ufs: fix fragment relocation offsets within a folio
  ufs: revalidate filesystem state before remounting read-write
  ufs: refuse read-write mount when fsck or journal replay is needed
  ufs: refuse read-write mount of filesystems with active snapshots
  ufs: refuse read-write mount of check-hashed filesystems

 fs/ufs/balloc.c |   8 +--
 fs/ufs/ialloc.c |   4 +-
 fs/ufs/inode.c  | 126 +++++++++++++++++++++++++++++++++++++-------
 fs/ufs/super.c  | 136 +++++++++++++++++++++++++++++++++++-------------
 fs/ufs/ufs.h    |   2 +
 fs/ufs/ufs_fs.h |  33 +++++++++++-
 6 files changed, 246 insertions(+), 63 deletions(-)


base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
-- 
2.55.0


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

* [PATCH 01/10] ufs: name the modern UFS2 superblock fields
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 02/10] ufs: use i_size to detect fast symlinks Ali Ahmet Memis
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

The 4.4BSD derived superblock that UFS2 uses has grown a number of
fields since Linux last looked at it. All of them currently sit inside
fs_44.fs_sparecon[50], so nothing in fs/ufs can tell that a filesystem
uses snapshots, soft updates journalling or metadata check hashes.

Split fs_sparecon[] into the fields FreeBSD defines, counting from
fs_pendinginodes: fs_snapinum[20], fs_avgfilesize, fs_avgfpdir,
fs_available_spare, fs_mtime, fs_sujfree, 21 remaining spare words,
fs_ckhash, fs_metackhash and the 32 bit fs_flags, followed by the
already named fs_contigsumsize. That is exactly 50 words, so the layout
does not change; sizeof(struct ufs_super_block_third) stays 356 and
fs_contigsumsize stays at offset 292.

The 8 bit fs_flags next to fs_clean is a different field, FreeBSD's
fs_old_flags. Rename it accordingly so the two cannot be confused. It
has no users.

Add the UFS_FS_* values for the 32 bit fs_flags and UFS_FSMAXSNAP.
No functional change; later patches use these.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/ufs_fs.h | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/fs/ufs/ufs_fs.h b/fs/ufs/ufs_fs.h
index b8dc354ae90f..32925c356fa2 100644
--- a/fs/ufs/ufs_fs.h
+++ b/fs/ufs/ufs_fs.h
@@ -154,6 +154,26 @@ typedef __u16 __bitwise __fs16;
 #define UFS_FSLOG     ((__s8)0xfd)	/* logging fs */
 #define UFS_FSFIX     ((__s8)0xfc)	/* being repaired while mounted */
 
+/* maximum number of snapshots recorded in fs_snapinum */
+#define UFS_FSMAXSNAP 20
+
+/*
+ * Values for the 32 bit fs_flags of the modern (4.4BSD derived) superblock.
+ * Not to be confused with the historic 8 bit fs_old_flags.
+ */
+#define UFS_FS_UNCLEAN		0x00000001
+#define UFS_FS_DOSOFTDEP	0x00000002
+#define UFS_FS_NEEDSFSCK	0x00000004
+#define UFS_FS_SUJ		0x00000008
+#define UFS_FS_ACLS		0x00000010
+#define UFS_FS_MULTILABEL	0x00000020
+#define UFS_FS_GJOURNAL		0x00000040
+#define UFS_FS_FLAGS_UPDATED	0x00000080
+#define UFS_FS_NFS4ACLS		0x00000100
+#define UFS_FS_METACKHASH	0x00000200
+#define UFS_FS_TRIM		0x00000400
+#define UFS_FS_INDEXDIRS	0x01000000
+
 /* From here to next blank line, s_flags for ufs_sb_info */
 /* directory entry encoding */
 #define UFS_DE_MASK		0x00000010	/* mask for the following */
@@ -865,7 +885,7 @@ struct ufs_super_block_first {
 	__s8	fs_fmod;
 	__s8	fs_clean;
 	__s8	fs_ronly;
-	__s8	fs_flags;
+	__s8	fs_old_flags;
 	__s8	fs_fsmnt[UFS_MAXMNTLEN - 212];
 
 };
@@ -937,7 +957,16 @@ struct ufs_super_block_third {
 			__fs32	fs_qfmask[2];	/* ~usb_fmask */
 		} fs_sunx86;
 		struct {
-			__fs32	fs_sparecon[50];/* reserved for future constants */
+			__fs32	fs_snapinum[UFS_FSMAXSNAP];/* snapshot inode numbers */
+			__fs32	fs_avgfilesize;	/* expected average file size */
+			__fs32	fs_avgfpdir;	/* expected # of files per directory */
+			__fs32	fs_available_spare;/* old scratch space */
+			__fs32	fs_mtime[2];	/* last mount or fsck time */
+			__fs32	fs_sujfree;	/* SUJ free list */
+			__fs32	fs_sparecon[21];/* reserved for future constants */
+			__fs32	fs_ckhash;	/* if CK_SUPERBLOCK, its check-hash */
+			__fs32	fs_metackhash;	/* metadata check-hash, see CK_ */
+			__fs32	fs_flags;	/* see UFS_FS_* above */
 			__fs32	fs_contigsumsize;/* size of cluster summary array */
 			__fs32	fs_maxsymlinklen;/* max length of an internal symlink */
 			__fs32	fs_inodefmt;	/* format of on-disk inodes */
-- 
2.55.0


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

* [PATCH 02/10] ufs: use i_size to detect fast symlinks
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 01/10] ufs: name the modern UFS2 superblock fields Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 03/10] ufs: free UFS2 external attribute blocks on inode deletion Ali Ahmet Memis
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

Whether a symlink stores its target inline is decided by testing
i_blocks == 0. The on-disk criterion is i_size < fs_maxsymlinklen;
FreeBSD's ufs_readlink() and ufs_symlink() both use it, and our own
ufs_symlink() already checks s_maxsymlinklen on the write side.

The two disagree on UFS2 inodes with extended attributes. ffs_alloc()
adds every allocation to di_blocks, including the IO_EXT path, so a
symlink carrying an extended attribute, an ACL or a MAC label has
di_blocks != 0 while its target is still stored inline.

Linux then takes it for a slow symlink. readlink() reads the target
through ufs_getfrag_block(), treating the ASCII bytes of the target as
block pointers. On a read-write mount unlinking it reaches
ufs_truncate_blocks() from ufs_evict_inode(), which hands the same bytes
to ufs_free_fragments() as block numbers. Values built from printable
characters fall outside the device, so this ends in ufs_panic() with the
inode already modified.

Add ufs_is_fast_symlink() and use it in the read, write and evict paths.
In ufs_evict_inode() it has to run before i_size is cleared.

Flavours that leave s_maxsymlinklen at 0 keep the old i_blocks
heuristic, so only 44BSD and UFS2 change behaviour.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/inode.c | 54 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index 440d014cc5ed..802bc6e01db2 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -517,6 +517,17 @@ const struct address_space_operations ufs_aops = {
 	.bmap = ufs_bmap
 };
 
+// on-disk criterion is i_size < fs_maxsymlinklen, not i_blocks
+static bool ufs_is_fast_symlink(struct inode *inode)
+{
+	struct ufs_sb_private_info *uspi = UFS_SB(inode->i_sb)->s_uspi;
+
+	if (uspi->s_maxsymlinklen)
+		return inode->i_size < uspi->s_maxsymlinklen;
+
+	return inode->i_blocks == 0;
+}
+
 static void ufs_set_inode_ops(struct inode *inode)
 {
 	if (S_ISREG(inode->i_mode)) {
@@ -528,7 +539,7 @@ static void ufs_set_inode_ops(struct inode *inode)
 		inode->i_fop = &ufs_dir_operations;
 		inode->i_mapping->a_ops = &ufs_aops;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (!inode->i_blocks) {
+		if (ufs_is_fast_symlink(inode)) {
 			inode->i_link = (char *)UFS_I(inode)->i_u1.i_symlink;
 			inode->i_op = &simple_symlink_inode_operations;
 		} else {
@@ -578,13 +589,13 @@ static int ufs1_read_inode(struct inode *inode, struct ufs_inode *ufs_inode)
 	ufsi->i_oeftflag = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_oeftflag);
 
 
-	if (S_ISCHR(mode) || S_ISBLK(mode) || inode->i_blocks) {
-		memcpy(ufsi->i_u1.i_data, &ufs_inode->ui_u2.ui_addr,
-		       sizeof(ufs_inode->ui_u2.ui_addr));
-	} else {
+	if (S_ISLNK(mode) && ufs_is_fast_symlink(inode)) {
 		memcpy(ufsi->i_u1.i_symlink, ufs_inode->ui_u2.ui_symlink,
 		       sizeof(ufs_inode->ui_u2.ui_symlink) - 1);
 		ufsi->i_u1.i_symlink[sizeof(ufs_inode->ui_u2.ui_symlink) - 1] = 0;
+	} else {
+		memcpy(ufsi->i_u1.i_data, &ufs_inode->ui_u2.ui_addr,
+		       sizeof(ufs_inode->ui_u2.ui_addr));
 	}
 	return 0;
 }
@@ -625,13 +636,13 @@ static int ufs2_read_inode(struct inode *inode, struct ufs2_inode *ufs2_inode)
 	ufsi->i_oeftflag = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_oeftflag);
 	*/
 
-	if (S_ISCHR(mode) || S_ISBLK(mode) || inode->i_blocks) {
-		memcpy(ufsi->i_u1.u2_i_data, &ufs2_inode->ui_u2.ui_addr,
-		       sizeof(ufs2_inode->ui_u2.ui_addr));
-	} else {
+	if (S_ISLNK(mode) && ufs_is_fast_symlink(inode)) {
 		memcpy(ufsi->i_u1.i_symlink, ufs2_inode->ui_u2.ui_symlink,
 		       sizeof(ufs2_inode->ui_u2.ui_symlink) - 1);
 		ufsi->i_u1.i_symlink[sizeof(ufs2_inode->ui_u2.ui_symlink) - 1] = 0;
+	} else {
+		memcpy(ufsi->i_u1.u2_i_data, &ufs2_inode->ui_u2.ui_addr,
+		       sizeof(ufs2_inode->ui_u2.ui_addr));
 	}
 	return 0;
 }
@@ -731,13 +742,12 @@ static void ufs1_update_inode(struct inode *inode, struct ufs_inode *ufs_inode)
 	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
 		/* ufs_inode->ui_u2.ui_addr.ui_db[0] = cpu_to_fs32(sb, inode->i_rdev); */
 		ufs_inode->ui_u2.ui_addr.ui_db[0] = ufsi->i_u1.i_data[0];
-	} else if (inode->i_blocks) {
-		memcpy(&ufs_inode->ui_u2.ui_addr, ufsi->i_u1.i_data,
-		       sizeof(ufs_inode->ui_u2.ui_addr));
-	}
-	else {
+	} else if (S_ISLNK(inode->i_mode) && ufs_is_fast_symlink(inode)) {
 		memcpy(&ufs_inode->ui_u2.ui_symlink, ufsi->i_u1.i_symlink,
 		       sizeof(ufs_inode->ui_u2.ui_symlink));
+	} else {
+		memcpy(&ufs_inode->ui_u2.ui_addr, ufsi->i_u1.i_data,
+		       sizeof(ufs_inode->ui_u2.ui_addr));
 	}
 
 	if (!inode->i_nlink)
@@ -774,13 +784,13 @@ static void ufs2_update_inode(struct inode *inode, struct ufs2_inode *ufs_inode)
 	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
 		/* ufs_inode->ui_u2.ui_addr.ui_db[0] = cpu_to_fs32(sb, inode->i_rdev); */
 		ufs_inode->ui_u2.ui_addr.ui_db[0] = ufsi->i_u1.u2_i_data[0];
-	} else if (inode->i_blocks) {
-		memcpy(&ufs_inode->ui_u2.ui_addr, ufsi->i_u1.u2_i_data,
-		       sizeof(ufs_inode->ui_u2.ui_addr));
-	} else {
+	} else if (S_ISLNK(inode->i_mode) && ufs_is_fast_symlink(inode)) {
 		memcpy(&ufs_inode->ui_u2.ui_symlink, ufsi->i_u1.i_symlink,
 		       sizeof(ufs_inode->ui_u2.ui_symlink));
- 	}
+	} else {
+		memcpy(&ufs_inode->ui_u2.ui_addr, ufsi->i_u1.u2_i_data,
+		       sizeof(ufs_inode->ui_u2.ui_addr));
+	}
 
 	if (!inode->i_nlink)
 		memset (ufs_inode, 0, sizeof(struct ufs2_inode));
@@ -839,14 +849,18 @@ int ufs_sync_inode (struct inode *inode)
 void ufs_evict_inode(struct inode * inode)
 {
 	int want_delete = 0;
+	bool fast_symlink;
 
 	if (!inode->i_nlink && !is_bad_inode(inode))
 		want_delete = 1;
 
+	// must be evaluated before i_size is cleared below
+	fast_symlink = S_ISLNK(inode->i_mode) && ufs_is_fast_symlink(inode);
+
 	truncate_inode_pages_final(&inode->i_data);
 	if (want_delete) {
 		inode->i_size = 0;
-		if (inode->i_blocks &&
+		if (inode->i_blocks && !fast_symlink &&
 		    (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 		     S_ISLNK(inode->i_mode)))
 			ufs_truncate_blocks(inode);
-- 
2.55.0


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

* [PATCH 03/10] ufs: free UFS2 external attribute blocks on inode deletion
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 01/10] ufs: name the modern UFS2 superblock fields Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 02/10] ufs: use i_size to detect fast symlinks Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 04/10] ufs: do not inherit file flags from the parent directory Ali Ahmet Memis
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

A UFS2 inode can own up to UFS_NXADDR blocks holding extended
attributes, described by di_extsize and di_extb[]. They sit outside the
block tree of the file, but ffs_alloc() counts them in di_blocks like
any other allocation.

Linux never reads those two fields. ufs_truncate_blocks() only walks
di_db[] and di_ib[], so on unlink the extattr blocks stay marked in use
while ufs_update_inode() clears the whole on-disk inode, di_extb[]
included. The only record of where the blocks were is gone, and nothing
can ever free them. Deleting FreeBSD files that carry an extended
attribute, an ACL or a MAC label therefore leaks disk space until fsck
reclaims it.

Keep di_extsize and di_extb[] in the in-core inode and release them from
ufs_evict_inode(), before the on-disk inode is cleared. The last block
of the area can be a fragment, so size it the way FreeBSD's sblksize()
does rather than assuming a full block.

An external attribute size larger than the area can describe means the
inode is corrupted and the block numbers cannot be trusted. Warn and
leave the blocks alone in that case; leaking them is better than freeing
blocks that may belong to something else.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/ialloc.c |  2 ++
 fs/ufs/inode.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ufs/ufs.h    |  2 ++
 3 files changed, 59 insertions(+)

diff --git a/fs/ufs/ialloc.c b/fs/ufs/ialloc.c
index 8e51f4630d18..4c3c76c4e7ff 100644
--- a/fs/ufs/ialloc.c
+++ b/fs/ufs/ialloc.c
@@ -294,6 +294,8 @@ struct inode *ufs_new_inode(struct inode *dir, umode_t mode)
 	inode->i_generation = 0;
 	simple_inode_init_ts(inode);
 	ufsi->i_flags = UFS_I(dir)->i_flags;
+	ufsi->i_extsize = 0;
+	memset(ufsi->i_extb, 0, sizeof(ufsi->i_extb));
 	ufsi->i_lastfrag = 0;
 	ufsi->i_shadow = 0;
 	ufsi->i_osync = 0;
diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index 802bc6e01db2..7079285bac2a 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -585,6 +585,8 @@ static int ufs1_read_inode(struct inode *inode, struct ufs_inode *ufs_inode)
 	inode->i_blocks = fs32_to_cpu(sb, ufs_inode->ui_blocks);
 	inode->i_generation = fs32_to_cpu(sb, ufs_inode->ui_gen);
 	ufsi->i_flags = fs32_to_cpu(sb, ufs_inode->ui_flags);
+	ufsi->i_extsize = 0;
+	memset(ufsi->i_extb, 0, sizeof(ufsi->i_extb));
 	ufsi->i_shadow = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_shadow);
 	ufsi->i_oeftflag = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_oeftflag);
 
@@ -605,6 +607,7 @@ static int ufs2_read_inode(struct inode *inode, struct ufs2_inode *ufs2_inode)
 	struct ufs_inode_info *ufsi = UFS_I(inode);
 	struct super_block *sb = inode->i_sb;
 	umode_t mode;
+	unsigned int i;
 
 	UFSD("Reading ufs2 inode, ino %llu\n", inode->i_ino);
 	/*
@@ -631,6 +634,9 @@ static int ufs2_read_inode(struct inode *inode, struct ufs2_inode *ufs2_inode)
 	inode->i_blocks = fs64_to_cpu(sb, ufs2_inode->ui_blocks);
 	inode->i_generation = fs32_to_cpu(sb, ufs2_inode->ui_gen);
 	ufsi->i_flags = fs32_to_cpu(sb, ufs2_inode->ui_flags);
+	ufsi->i_extsize = fs32_to_cpu(sb, ufs2_inode->ui_extsize);
+	for (i = 0; i < UFS_NXADDR; i++)
+		ufsi->i_extb[i] = fs64_to_cpu(sb, ufs2_inode->ui_extb[i]);
 	/*
 	ufsi->i_shadow = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_shadow);
 	ufsi->i_oeftflag = fs32_to_cpu(sb, ufs_inode->ui_u3.ui_sun.ui_oeftflag);
@@ -846,6 +852,54 @@ int ufs_sync_inode (struct inode *inode)
 	return ufs_update_inode (inode, 1);
 }
 
+/*
+ * UFS2 keeps extended attributes in up to UFS_NXADDR blocks of their own.
+ * They live outside the block tree of the file but are counted in di_blocks.
+ * Linux does not implement extended attributes, but it still has to release
+ * those blocks when the inode goes away: ufs_update_inode() clears the whole
+ * on-disk inode, taking di_extb[] with it, so nothing would ever free them.
+ */
+static void ufs_free_ext_blocks(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	struct ufs_sb_private_info *uspi = UFS_SB(sb)->s_uspi;
+	struct ufs_inode_info *ufsi = UFS_I(inode);
+	u64 size = ufsi->i_extsize;
+	unsigned int i;
+
+	if ((UFS_SB(sb)->s_flags & UFS_TYPE_MASK) != UFS_TYPE_UFS2 || !size)
+		return;
+
+	if (size > ((u64)UFS_NXADDR << uspi->s_bshift)) {
+		// leaking is better than freeing something we cannot locate
+		ufs_warning(sb, __func__,
+			    "inode %llu: bad external attribute size %llu\n",
+			    inode->i_ino, size);
+		return;
+	}
+
+	for (i = 0; i < UFS_NXADDR; i++) {
+		u64 off = (u64)i << uspi->s_bshift;
+		u64 block = ufsi->i_extb[i];
+		unsigned int frags;
+
+		if (!block || size <= off)
+			continue;
+
+		if (size - off >= uspi->s_bsize)
+			frags = uspi->s_fpb;
+		else
+			frags = (size - off + uspi->s_fsize - 1) >> uspi->s_fshift;
+
+		ufsi->i_extb[i] = 0;
+		if (frags == uspi->s_fpb)
+			ufs_free_blocks(inode, block, frags);
+		else
+			ufs_free_fragments(inode, block, frags);
+	}
+	ufsi->i_extsize = 0;
+}
+
 void ufs_evict_inode(struct inode * inode)
 {
 	int want_delete = 0;
@@ -864,6 +918,7 @@ void ufs_evict_inode(struct inode * inode)
 		    (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 		     S_ISLNK(inode->i_mode)))
 			ufs_truncate_blocks(inode);
+		ufs_free_ext_blocks(inode);
 		ufs_update_inode(inode, inode_needs_sync(inode));
 	}
 
diff --git a/fs/ufs/ufs.h b/fs/ufs/ufs.h
index 788e025056b2..6cac5f8222eb 100644
--- a/fs/ufs/ufs.h
+++ b/fs/ufs/ufs.h
@@ -40,6 +40,8 @@ struct ufs_inode_info {
 		__fs64	u2_i_data[15];
 	} i_u1;
 	__u32	i_flags;
+	__u32	i_extsize;		/* UFS2 external attribute area size */
+	__u64	i_extb[UFS_NXADDR];	/* UFS2 external attribute blocks */
 	__u32	i_shadow;
 	__u32	i_unused1;
 	__u32	i_unused2;
-- 
2.55.0


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

* [PATCH 04/10] ufs: do not inherit file flags from the parent directory
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (2 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 03/10] ufs: free UFS2 external attribute blocks on inode deletion Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 05/10] ufs: honour on-disk immutable and append-only flags Ali Ahmet Memis
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

ufs_new_inode() starts a new inode off with the parent directory's
on-disk flags:

	ufsi->i_flags = UFS_I(dir)->i_flags;

These are BSD chflags, and they have no inheritance semantics. FreeBSD's
ufs_makeinode() leaves di_flags at zero and only ever sets UF_OPAQUE, and
only for a whiteout. The line comes from the ext2 code fs/ufs/ialloc.c
was derived from, where the flags are Linux inode flags that do have an
inherited subset.

The result is that a file created on Linux inside a directory FreeBSD
marked with chflags schg or sappnd is written out carrying SF_IMMUTABLE
or SF_APPEND itself. Linux does not act on those bits, so nothing looks
wrong at the time, but back on FreeBSD the new file is immutable or
append-only and cannot be removed without lowering the securelevel.
Nothing asked for that.

Start new inodes with no flags set.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/ialloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ufs/ialloc.c b/fs/ufs/ialloc.c
index 4c3c76c4e7ff..dc3eeb415876 100644
--- a/fs/ufs/ialloc.c
+++ b/fs/ufs/ialloc.c
@@ -293,7 +293,7 @@ struct inode *ufs_new_inode(struct inode *dir, umode_t mode)
 	inode->i_blocks = 0;
 	inode->i_generation = 0;
 	simple_inode_init_ts(inode);
-	ufsi->i_flags = UFS_I(dir)->i_flags;
+	ufsi->i_flags = 0;
 	ufsi->i_extsize = 0;
 	memset(ufsi->i_extb, 0, sizeof(ufsi->i_extb));
 	ufsi->i_lastfrag = 0;
-- 
2.55.0


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

* [PATCH 05/10] ufs: honour on-disk immutable and append-only flags
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (3 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 04/10] ufs: do not inherit file flags from the parent directory Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 06/10] ufs: fix fragment relocation offsets within a folio Ali Ahmet Memis
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

ui_flags holds the BSD chflags of the inode. ufs1_read_inode() and
ufs2_read_inode() copy it into ufsi->i_flags and nothing else ever looks
at it. In particular it never reaches inode->i_flags, so IS_IMMUTABLE()
and IS_APPEND() are always false for a UFS inode. The check in
ufs_truncate(),

	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
		return -EPERM;

can never fire, and neither can the generic ones in the VFS.

A file that FreeBSD marked with chflags schg or uchg is therefore freely
writable and removable once the filesystem is mounted on Linux, and an
sappnd file can be rewritten rather than only appended to.

Map the immutable and append-only bits onto S_IMMUTABLE and S_APPEND
when the inode is read. Only 44BSD and UFS2 are handled: the other
flavours put something else at that offset and their ui_flags is not a
chflags word.

NOUNLINK has no equivalent in inode->i_flags. Mapping it to S_IMMUTABLE
would be wrong, since it is meant to allow modification, so files marked
that way stay removable for now.

Note that Linux offers no way to clear these flags again; UFS has no
FS_IOC_SETFLAGS support. Filesystems carrying them have to be edited
from an operating system that implements chflags.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/inode.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index 7079285bac2a..029f6771b214 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -517,6 +517,22 @@ const struct address_space_operations ufs_aops = {
 	.bmap = ufs_bmap
 };
 
+// only 44BSD and UFS2 store BSD chflags in ui_flags
+static void ufs_set_inode_flags(struct inode *inode)
+{
+	unsigned int flavour = UFS_SB(inode->i_sb)->s_flavour;
+	unsigned int flags = UFS_I(inode)->i_flags;
+
+	if (flavour != UFS_MOUNT_UFSTYPE_44BSD &&
+	    flavour != UFS_MOUNT_UFSTYPE_UFS2)
+		return;
+
+	if (flags & (UFS_UF_IMMUTABLE | UFS_SF_IMMUTABLE))
+		inode->i_flags |= S_IMMUTABLE;
+	if (flags & (UFS_UF_APPEND | UFS_SF_APPEND))
+		inode->i_flags |= S_APPEND;
+}
+
 // on-disk criterion is i_size < fs_maxsymlinklen, not i_blocks
 static bool ufs_is_fast_symlink(struct inode *inode)
 {
@@ -704,6 +720,7 @@ struct inode *ufs_iget(struct super_block *sb, unsigned long ino)
 	ufsi->i_dir_start_lookup = 0;
 	ufsi->i_osync = 0;
 
+	ufs_set_inode_flags(inode);
 	ufs_set_inode_ops(inode);
 
 	UFSD("EXIT\n");
-- 
2.55.0


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

* [PATCH 06/10] ufs: fix fragment relocation offsets within a folio
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (4 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 05/10] ufs: honour on-disk immutable and append-only flags Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 07/10] ufs: revalidate filesystem state before remounting read-write Ali Ahmet Memis
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner
  Cc: Jan Kara, linux-fsdevel, linux-kernel, stable

ufs_change_blocknr() walks the buffers covering the fragments being
moved from oldb to newb. Inside a folio it starts at the buffer holding
the first fragment:

	pos = i & mask;
	for (j = 0; j < pos; ++j)
		bh = bh->b_this_page;

so j is a buffer index within the folio, not an offset into the range
being relocated. The body then uses it as if it were one:

	pos = (i - beg) + j;

On the first folio i equals beg, so pos comes out as beg & mask where it
should be zero. Every fragment is then shifted by that amount, both the
source it is read from, oldb + pos, and the destination recorded in
bh->b_blocknr, newb + pos. The tail of the relocation ends up past the
end of the new allocation, which is only count fragments long, leaving
dirty buffers pointing at fragments that can belong to another file or
to metadata. Later folios start folio aligned and are unaffected.

Reaching it needs a page larger than the filesystem block. beg is the
start of the existing tail and so is always a multiple of fs_frag, while
mask is blks_per_page - 1. When PAGE_SIZE is at most fs_bsize the two
are powers of two with fs_frag the larger, beg & mask is zero and
nothing goes wrong. ufs_fill_super() already refuses fs_bsize below
4096, so every 4 KiB page system is in that case. A 16 KiB or 64 KiB
page kernel, as used on arm64 and ppc64, mounting a filesystem with a
smaller block is not.

Keep the starting buffer index and subtract it, so pos counts from the
start of the relocated range again.

Reproduced on an arm64 64 KiB page kernel with a 4096/512 UFS1
filesystem, where a block is 8 fragments and blks_per_page is 128. A
file with a one fragment tail in its second block has beg 8, so
beg & mask is 8. Filling the fragments that physically follow that tail
and then extending the file forces the relocation:

  ufs: RELOC: beg=8 count=1 oldb=1457 newb=1528 mask=127 beg&mask=8

Before this patch the first byte of the relocated tail reads back as 0
instead of the written data; after it the file compares equal. The same
test on a 4 KiB page kernel never gets beg & mask nonzero, as expected.

Fixes: 5431bf97ce69 ("[PATCH] ufs: prepare write + change blocks on the fly")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/balloc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ufs/balloc.c b/fs/ufs/balloc.c
index 628edfde3a9f..f617df64c45b 100644
--- a/fs/ufs/balloc.c
+++ b/fs/ufs/balloc.c
@@ -241,7 +241,7 @@ static void ufs_change_blocknr(struct inode *inode, sector_t beg,
 	const unsigned mask = blks_per_page - 1;
 	struct address_space * const mapping = inode->i_mapping;
 	pgoff_t index, cur_index, last_index;
-	unsigned pos, j, lblock;
+	unsigned int pos, j, lblock, first;
 	sector_t end, i;
 	struct buffer_head *head, *bh;
 
@@ -272,8 +272,8 @@ static void ufs_change_blocknr(struct inode *inode, sector_t beg,
 
 		head = folio_buffers(folio);
 		bh = head;
-		pos = i & mask;
-		for (j = 0; j < pos; ++j)
+		first = i & mask;
+		for (j = 0; j < first; ++j)
 			bh = bh->b_this_page;
 
 		if (unlikely(index == last_index))
@@ -284,7 +284,7 @@ static void ufs_change_blocknr(struct inode *inode, sector_t beg,
 		do {
 			if (j >= lblock)
 				break;
-			pos = (i - beg) + j;
+			pos = (i - beg) + (j - first);
 
 			if (!buffer_mapped(bh))
 					map_bh(bh, inode->i_sb, oldb + pos);
-- 
2.55.0


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

* [PATCH 07/10] ufs: revalidate filesystem state before remounting read-write
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (5 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 06/10] ufs: fix fragment relocation offsets within a folio Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 08/10] ufs: refuse read-write mount when fsck or journal replay is needed Ali Ahmet Memis
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

ufs_fill_super() inspects fs_clean and, for the Solaris flavours, the
state timestamp, and forces SB_RDONLY when the filesystem was not shut
down cleanly. The remount path never repeats that work. Going from
read-only to read-write only checks that the flavour has write support
at all and that the cylinder groups can be read:

	if (!ufs_read_cylinder_structures(sb)) {
		...
	}
	sb->s_flags &= ~SB_RDONLY;

So the decision taken at mount time can simply be undone:

	mount -t ufs -o ufstype=ufs2,rw bad.img /mnt   # forced read-only
	mount -o remount,rw /mnt                       # writable again
	touch /mnt/x

A filesystem that is active, bad or in need of fsck becomes writable
after all, which makes the check at mount time close to meaningless.

Move the state test into ufs_state_allows_write() and call it from both
places, returning -EROFS from the remount when it fails. Linux never
writes fs_clean itself outside of the error paths, so the value the
helper sees on remount is still the one that came off the disk.

No change in which filesystems are accepted at mount time.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/super.c | 95 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index c4831a8b9b3f..4b0e9196fa41 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -713,6 +713,60 @@ static u64 ufs_max_bytes(struct super_block *sb)
 	return res << uspi->s_bshift;
 }
 
+/*
+ * Does the recorded state of the filesystem allow us to write to it?
+ * Called both when mounting and when remounting read-write.
+ */
+static bool ufs_state_allows_write(struct super_block *sb)
+{
+	struct ufs_sb_private_info *uspi = UFS_SB(sb)->s_uspi;
+	struct ufs_super_block_first *usb1 = ubh_get_usb_first(uspi);
+	struct ufs_super_block_third *usb3 = ubh_get_usb_third(uspi);
+
+	switch (UFS_SB(sb)->s_flags & UFS_ST_MASK) {
+	case UFS_ST_44BSD:
+	case UFS_ST_OLD:
+		break;
+	case UFS_ST_SUN:
+	case UFS_ST_SUNOS:
+	case UFS_ST_SUNx86:
+		if (ufs_get_fs_state(sb, usb1, usb3) !=
+		    UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time)) {
+			pr_err("%s(): fs needs fsck\n", __func__);
+			return false;
+		}
+		break;
+	default:
+		pr_err("%s(): fs needs fsck\n", __func__);
+		return false;
+	}
+
+	switch (usb1->fs_clean) {
+	case UFS_FSCLEAN:
+		UFSD("fs is clean\n");
+		return true;
+	case UFS_FSSTABLE:
+		UFSD("fs is stable\n");
+		return true;
+	case UFS_FSLOG:
+		UFSD("fs is logging fs\n");
+		return true;
+	case UFS_FSOSF1:
+		UFSD("fs is DEC OSF/1\n");
+		return true;
+	case UFS_FSACTIVE:
+		pr_err("%s(): fs is active\n", __func__);
+		return false;
+	case UFS_FSBAD:
+		pr_err("%s(): fs is bad\n", __func__);
+		return false;
+	default:
+		pr_err("%s(): can't grok fs_clean 0x%x\n",
+		       __func__, usb1->fs_clean);
+		return false;
+	}
+}
+
 static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct ufs_fs_context *ctx = fc->fs_private;
@@ -1049,43 +1103,8 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 	 * Check, if file system was correctly unmounted.
 	 * If not, make it read only.
 	 */
-	if (((flags & UFS_ST_MASK) == UFS_ST_44BSD) ||
-	  ((flags & UFS_ST_MASK) == UFS_ST_OLD) ||
-	  (((flags & UFS_ST_MASK) == UFS_ST_SUN ||
-	    (flags & UFS_ST_MASK) == UFS_ST_SUNOS ||
-	  (flags & UFS_ST_MASK) == UFS_ST_SUNx86) &&
-	  (ufs_get_fs_state(sb, usb1, usb3) == (UFS_FSOK - fs32_to_cpu(sb, usb1->fs_time))))) {
-		switch(usb1->fs_clean) {
-		case UFS_FSCLEAN:
-			UFSD("fs is clean\n");
-			break;
-		case UFS_FSSTABLE:
-			UFSD("fs is stable\n");
-			break;
-		case UFS_FSLOG:
-			UFSD("fs is logging fs\n");
-			break;
-		case UFS_FSOSF1:
-			UFSD("fs is DEC OSF/1\n");
-			break;
-		case UFS_FSACTIVE:
-			pr_err("%s(): fs is active\n", __func__);
-			sb->s_flags |= SB_RDONLY;
-			break;
-		case UFS_FSBAD:
-			pr_err("%s(): fs is bad\n", __func__);
-			sb->s_flags |= SB_RDONLY;
-			break;
-		default:
-			pr_err("%s(): can't grok fs_clean 0x%x\n",
-			       __func__, usb1->fs_clean);
-			sb->s_flags |= SB_RDONLY;
-			break;
-		}
-	} else {
-		pr_err("%s(): fs needs fsck\n", __func__);
+	if (!ufs_state_allows_write(sb))
 		sb->s_flags |= SB_RDONLY;
-	}
 
 	/*
 	 * Read ufs_super_block into internal data structures
@@ -1291,6 +1310,10 @@ static int ufs_reconfigure(struct fs_context *fc)
 			mutex_unlock(&UFS_SB(sb)->s_lock);
 			return -EINVAL;
 		}
+		if (!ufs_state_allows_write(sb)) {
+			mutex_unlock(&UFS_SB(sb)->s_lock);
+			return -EROFS;
+		}
 		if (!ufs_read_cylinder_structures(sb)) {
 			pr_err("failed during remounting\n");
 			mutex_unlock(&UFS_SB(sb)->s_lock);
-- 
2.55.0


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

* [PATCH 08/10] ufs: refuse read-write mount when fsck or journal replay is needed
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (6 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 07/10] ufs: revalidate filesystem state before remounting read-write Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 09/10] ufs: refuse read-write mount of filesystems with active snapshots Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 10/10] ufs: refuse read-write mount of check-hashed filesystems Ali Ahmet Memis
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

FS_NEEDSFSCK, FS_SUJ and FS_GJOURNAL live in the 32 bit fs_flags of the
modern superblock, which Linux has never read. A UFS2 filesystem that
FreeBSD marked as needing a foreground fsck, or that carries soft
updates journalling or a GEOM journal, therefore mounts read-write here
and is written to with the pending recovery work still outstanding.

FreeBSD itself will not do this. ffs_mountfs() only accepts an unclean
filesystem when neither flag is set:

	(fs->fs_flags & (FS_SUJ | FS_NEEDSFSCK)) == 0 &&
	    (fs->fs_flags & FS_DOSOFTDEP)

and otherwise refuses with "Filesystem is not clean - run fsck".

Linux has no journal replay for either journal and no way to run the
recovery, so the only safe thing it can do is stay read-only. Check the
flags on mount and on remount read-write.

Only UFS2 is examined. On older layouts the same offset is scratch space
that may hold anything, and misreading it would wrongly refuse
filesystems that are perfectly fine.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/super.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 4b0e9196fa41..df95502e039e 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -767,6 +767,34 @@ static bool ufs_state_allows_write(struct super_block *sb)
 	}
 }
 
+/*
+ * Refuse to write to a filesystem using features we do not implement.
+ * Only UFS2 is examined: the 32 bit fs_flags lives in the modern part of
+ * the superblock, which older layouts leave as scratch space.
+ */
+static bool ufs_features_allow_write(struct super_block *sb)
+{
+	struct ufs_sb_private_info *uspi = UFS_SB(sb)->s_uspi;
+	struct ufs_super_block_third *usb3 = ubh_get_usb_third(uspi);
+	u32 fsflags;
+
+	if (uspi->fs_magic != UFS2_MAGIC)
+		return true;
+
+	fsflags = fs32_to_cpu(sb, usb3->fs_un2.fs_44.fs_flags);
+
+	if (fsflags & UFS_FS_NEEDSFSCK) {
+		pr_err("%s(): fs is marked as needing fsck\n", __func__);
+		return false;
+	}
+	if (fsflags & (UFS_FS_SUJ | UFS_FS_GJOURNAL)) {
+		pr_err("%s(): journalled fs, journal replay is not supported\n",
+		       __func__);
+		return false;
+	}
+	return true;
+}
+
 static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct ufs_fs_context *ctx = fc->fs_private;
@@ -1103,7 +1131,7 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 	 * Check, if file system was correctly unmounted.
 	 * If not, make it read only.
 	 */
-	if (!ufs_state_allows_write(sb))
+	if (!ufs_state_allows_write(sb) || !ufs_features_allow_write(sb))
 		sb->s_flags |= SB_RDONLY;
 
 	/*
@@ -1310,7 +1338,8 @@ static int ufs_reconfigure(struct fs_context *fc)
 			mutex_unlock(&UFS_SB(sb)->s_lock);
 			return -EINVAL;
 		}
-		if (!ufs_state_allows_write(sb)) {
+		if (!ufs_state_allows_write(sb) ||
+		    !ufs_features_allow_write(sb)) {
 			mutex_unlock(&UFS_SB(sb)->s_lock);
 			return -EROFS;
 		}
-- 
2.55.0


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

* [PATCH 09/10] ufs: refuse read-write mount of filesystems with active snapshots
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (7 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 08/10] ufs: refuse read-write mount when fsck or journal replay is needed Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  2026-08-01 22:55 ` [PATCH 10/10] ufs: refuse read-write mount of check-hashed filesystems Ali Ahmet Memis
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

A UFS2 filesystem can hold up to FSMAXSNAP snapshots, recorded in
fs_snapinum[]. A snapshot is an inode whose block pointers describe the
state of the filesystem at the moment it was taken, using the reserved
values BLK_NOCOPY and BLK_SNAP alongside ordinary block numbers.
Keeping one valid requires copy on write: before a block that the
snapshot references is overwritten or freed, its old contents have to be
copied into the snapshot.

Linux implements none of this. It does not read fs_snapinum[], does not
know the SF_SNAPSHOT inode flag, and treats the reserved block pointer
values 1 and 2 as ordinary fragment addresses. Mounting such a
filesystem read-write and changing a single file is enough for every
snapshot on it to stop describing the point in time it was taken at, and
there is no way to notice afterwards.

Refuse the read-write mount while any snapshot is recorded. Read-only
access is unaffected.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/super.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index df95502e039e..6132c28c4308 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -776,6 +776,7 @@ static bool ufs_features_allow_write(struct super_block *sb)
 {
 	struct ufs_sb_private_info *uspi = UFS_SB(sb)->s_uspi;
 	struct ufs_super_block_third *usb3 = ubh_get_usb_third(uspi);
+	unsigned int i;
 	u32 fsflags;
 
 	if (uspi->fs_magic != UFS2_MAGIC)
@@ -792,6 +793,12 @@ static bool ufs_features_allow_write(struct super_block *sb)
 		       __func__);
 		return false;
 	}
+	for (i = 0; i < UFS_FSMAXSNAP; i++) {
+		if (usb3->fs_un2.fs_44.fs_snapinum[i]) {
+			pr_err("%s(): fs has active snapshots\n", __func__);
+			return false;
+		}
+	}
 	return true;
 }
 
-- 
2.55.0


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

* [PATCH 10/10] ufs: refuse read-write mount of check-hashed filesystems
  2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
                   ` (8 preceding siblings ...)
  2026-08-01 22:55 ` [PATCH 09/10] ufs: refuse read-write mount of filesystems with active snapshots Ali Ahmet Memis
@ 2026-08-01 22:55 ` Ali Ahmet Memis
  9 siblings, 0 replies; 11+ messages in thread
From: Ali Ahmet Memis @ 2026-08-01 22:55 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner; +Cc: Jan Kara, linux-fsdevel, linux-kernel

Modern UFS2 protects metadata with check hashes. fs_metackhash says
which kinds are in use, CK_SUPERBLOCK, CK_CYLGRP, CK_INODE, CK_INDIR and
CK_DIR, and the hashes themselves live in fs_ckhash, cg_ckhash and
di_ckhash. FreeBSD recomputes them on every metadata write, for inodes
in ffs_update() right before the inode goes to disk, and treats a
mismatch as an integrity failure.

Linux knows about none of these fields. It happily allocates blocks,
changes inodes and rewrites cylinder groups without touching a single
hash, so an ordinary

	touch /mnt/x
	chmod 600 /mnt/x

leaves the filesystem valid by its own rules but failing every check
hash it advertises. Back on FreeBSD that surfaces as inode, cylinder
group and superblock check-hash failures on a filesystem that was never
damaged in any other way.

Implementing the hashes is a lot more work than this series is doing, so
stay read-only instead. Both the flag and a non-zero fs_metackhash are
required, so a filesystem that advertises the feature without actually
maintaining any hash is still writable.

This does change what userspace can do with an existing image: modern
FreeBSD enables check hashes by default, so filesystems that mount
read-write today will start coming up read-only. That is the point, but
it is a policy call rather than a plain bug fix, and it is the one patch
here that could be dropped without affecting the rest.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 6132c28c4308..52de393aebe2 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -799,6 +799,11 @@ static bool ufs_features_allow_write(struct super_block *sb)
 			return false;
 		}
 	}
+	if ((fsflags & UFS_FS_METACKHASH) &&
+	    fs32_to_cpu(sb, usb3->fs_un2.fs_44.fs_metackhash)) {
+		pr_err("%s(): fs uses metadata check hashes\n", __func__);
+		return false;
+	}
 	return true;
 }
 
-- 
2.55.0


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

end of thread, other threads:[~2026-08-01 22:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-01 22:55 [PATCH 00/10] ufs: correctness fixes for valid on-disk filesystems Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 01/10] ufs: name the modern UFS2 superblock fields Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 02/10] ufs: use i_size to detect fast symlinks Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 03/10] ufs: free UFS2 external attribute blocks on inode deletion Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 04/10] ufs: do not inherit file flags from the parent directory Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 05/10] ufs: honour on-disk immutable and append-only flags Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 06/10] ufs: fix fragment relocation offsets within a folio Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 07/10] ufs: revalidate filesystem state before remounting read-write Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 08/10] ufs: refuse read-write mount when fsck or journal replay is needed Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 09/10] ufs: refuse read-write mount of filesystems with active snapshots Ali Ahmet Memis
2026-08-01 22:55 ` [PATCH 10/10] ufs: refuse read-write mount of check-hashed filesystems Ali Ahmet Memis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®