WIP: fixup internal xattr usage

The roundtrip test currently fails with `special.dump`; I think this patch is in the right direction but...I think what we were writing into the EROFS superblock for the userxattr is wrong...which means fixing this in the obvious way is a format break. Signed-off-by: Colin Walters <[email protected]>
cgwalters · May 28, 2024 · 42af8db · 42af8db
1 parent f8b3660
commit 42af8db
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 4 deletions.
diff --git a/libcomposefs/lcfs-internal.h b/libcomposefs/lcfs-internal.h
@@ -67,6 +67,10 @@ typedef int errint_t;
 // trusted.overlay.opaque
 #define OVERLAY_XATTR_OPAQUE OVERLAY_XATTR_PREFIX "opaque"
 
+// user.overlay.overlay.
+#define OVERLAY_XATTR_USERXATTR_ESCAPE_PREFIX                                  \
+	OVERLAY_XATTR_USERXATTR_PREFIX "overlay."
+
 // trusted.overlay.overlay.whiteout
 #define OVERLAY_XATTR_ESCAPED_WHITEOUT OVERLAY_XATTR_ESCAPE_PREFIX "whiteout"
 // trusted.overlay.overlay.whiteouts
@@ -83,6 +87,10 @@ typedef int errint_t;
 // user.overlay.opaque
 #define OVERLAY_XATTR_USERXATTR_OPAQUE OVERLAY_XATTR_USERXATTR_PREFIX "opaque"
 
+// user.overlay.overlay.opaque
+#define OVERLAY_XATTR_USERXATTR_ESCAPED_OPAQUE                                 \
+	OVERLAY_XATTR_USERXATTR_ESCAPE_PREFIX "opaque"
+
 #define ALIGN_TO(_offset, _align_size)                                         \
 	(((_offset) + _align_size - 1) & ~(_align_size - 1))
 

diff --git a/libcomposefs/lcfs-writer-erofs.c b/libcomposefs/lcfs-writer-erofs.c
@@ -1150,14 +1150,17 @@ static int add_overlayfs_xattrs(struct lcfs_ctx_s *ctx, struct lcfs_node_s *node
 		if (ret < 0)
 			return ret;
 
-		/* Mark dir containing whiteouts with new format as of version 1 */
+		/* Mark dir containing whiteouts with new format as of version 1;
+		 * note the code in composefs-info.c which explicitly skips these.
+		 */
 		if (ctx->options->version >= 1) {
 			ret = lcfs_node_set_xattr(
 				parent, OVERLAY_XATTR_ESCAPED_OPAQUE, "x", 1);
 			if (ret < 0)
 				return ret;
 			ret = lcfs_node_set_xattr(
-				parent, OVERLAY_XATTR_USERXATTR_OPAQUE, "x", 1);
+				parent, OVERLAY_XATTR_USERXATTR_ESCAPED_OPAQUE,
+				"x", 1);
 			if (ret < 0)
 				return ret;
 		}
@@ -1522,6 +1525,7 @@ static int erofs_readdir_block(struct lcfs_image_data *data,
 	return 0;
 }
 
+// Convert an EROFS entry back into a composefs node.
 static int lcfs_build_node_erofs_xattr(struct lcfs_node_s *node, uint8_t name_index,
 				       const char *entry_name, uint8_t name_len,
 				       const char *value, uint16_t value_size)
@@ -1579,6 +1583,17 @@ static int lcfs_build_node_erofs_xattr(struct lcfs_node_s *node, uint8_t name_in
 			return 0;
 		}
 	}
+	if (str_has_prefix(name, OVERLAY_XATTR_USERXATTR_PREFIX)) {
+		if (str_has_prefix(name, OVERLAY_XATTR_USERXATTR_ESCAPE_PREFIX)) {
+			/* Unescape */
+			memmove(name + strlen(OVERLAY_XATTR_USER_PREFIX),
+				name + strlen(OVERLAY_XATTR_USERXATTR_PREFIX),
+				strlen(name) - strlen(OVERLAY_XATTR_USER_PREFIX) + 1);
+		} else {
+			/* skip */
+			return 0;
+		}
+	}
 
 	if (lcfs_node_set_xattr(node, name, value, value_size) < 0)
 		return -1;

diff --git a/tests/test-units.sh b/tests/test-units.sh
@@ -24,11 +24,12 @@ function test_dump_roundtrip () {
     testsrcdir=$(cd $(dirname $0) && pwd)
     pushd $1
     local src=$testsrcdir/assets/special.dump
+    set -x
     $BINDIR/mkcomposefs --from-file "${src}" out.cfs
     $BINDIR/composefs-info dump out.cfs > dump.txt
-    diff -u "${src}" dump.txt
+    diff -u "${src}" dump.txt || exit 1
     $BINDIR/mkcomposefs --from-file dump.txt out2.cfs
-    diff -u out.cfs out2.cfs
+    diff -u out.cfs out2.cfs || exit 1
     echo "ok dump roundtrip"
     popd
 }

diff --git a/tools/composefs-info.c b/tools/composefs-info.c
@@ -236,6 +236,22 @@ static void dump_node(struct lcfs_node_s *node, char *path)
 		size_t value_len;
 		const char *value = lcfs_node_get_xattr(target, name, &value_len);
 
+		// Skip the special xattrs injected by lcfs-writer-erofs.c to
+		// denote a directory containing whiteouts.
+		bool is_opaque_flag =
+			(strcmp(name, OVERLAY_XATTR_OPAQUE) == 0) ||
+			(strcmp(name, OVERLAY_XATTR_USERXATTR_OPAQUE) == 0);
+		bool is_special_x = value_len == 1 && *value == 'x';
+		if (is_opaque_flag && is_special_x) {
+			continue;
+		}
+
+		// Undo the effect of the escaping in add_overlayfs_xattrs
+		if (str_has_prefix(name, OVERLAY_XATTR_ESCAPE_PREFIX)) {
+			name += strlen(OVERLAY_XATTR_ESCAPE_PREFIX);
+		} else if (str_has_prefix(name, OVERLAY_XATTR_USERXATTR_ESCAPE_PREFIX)) {
+			name += strlen(OVERLAY_XATTR_USERXATTR_ESCAPE_PREFIX);
+		}
 		printf(" ");
 		print_escaped(name, -1, ESCAPE_EQUAL);
 		printf("=");