[PATCH 01/13] apfsck: verify NULL-termination of volume name

9 views
Skip to first unread message

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:02:37 PM3/25/19
to linux...@googlegroups.com
The name of a volume must fit in APFS_VOLNAME_LEN characters, and have
a NULL termination. Check this after mapping the volume superblock.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index ad68897..c9d7ad7 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -140,6 +140,7 @@ static struct apfs_superblock *map_volume_super(int vol,
struct volume_superblock *vsb)
{
struct apfs_nx_superblock *msb_raw = sb->s_raw;
+ char *vol_name;
u64 vol_id;

vol_id = le64_to_cpu(msb_raw->nx_fs_oid[vol]);
@@ -157,6 +158,11 @@ static struct apfs_superblock *map_volume_super(int vol,

vsb->v_next_obj_id = le64_to_cpu(vsb->v_raw->apfs_next_obj_id);
vsb->v_next_doc_id = le32_to_cpu(vsb->v_raw->apfs_next_doc_id);
+
+ vol_name = (char *)vsb->v_raw->apfs_volname;
+ if (strnlen(vol_name, APFS_VOLNAME_LEN) == APFS_VOLNAME_LEN)
+ report("Volume superblock", "name lacks NULL-termination.");
+
return vsb->v_raw;
}

--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:02:50 PM3/25/19
to linux...@googlegroups.com
The reserved field of the volume superblock may become used in the
future, but for now it should always be set to 0.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index c9d7ad7..a09c900 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -163,6 +163,9 @@ static struct apfs_superblock *map_volume_super(int vol,
if (strnlen(vol_name, APFS_VOLNAME_LEN) == APFS_VOLNAME_LEN)
report("Volume superblock", "name lacks NULL-termination.");

+ if (le16_to_cpu(vsb->v_raw->reserved) != 0)
+ report("Volume superblock", "reserved field is in use.");

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:03:00 PM3/25/19
to linux...@googlegroups.com
The volume superblock reports information about the software that
created and modified the volume. Check that it's internally consistent.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
apfsck/super.h | 2 +-
2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/apfsck/super.c b/apfsck/super.c
index a09c900..0f4fb86 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -129,6 +129,75 @@ static void map_main_super(void)
}

/**
+ * software_strlen - Calculate the length of a software info string
+ * @str: the string
+ *
+ * Also checks that the string has a proper null-termination, and only null
+ * characters afterwards.
+ */
+static int software_strlen(u8 *str)
+{
+ int length = strnlen((char *)str, APFS_MODIFIED_NAMELEN);
+ u8 *end = str + APFS_MODIFIED_NAMELEN;
+
+ if (length == APFS_MODIFIED_NAMELEN)
+ report("Volume software id", "no NULL-termination.");
+ for (str += length + 1; str != end; ++str) {
+ if (*str)
+ report("Volume software id", "goes on after NULL.");
+ }
+ return length;
+}
+
+/**
+ * check_software_info - Check the fields reporting implementation info
+ * @formatted_by: information about the software that created the volume
+ * @modified_by: information about the software that modified the volume
+ */
+static void check_software_information(struct apfs_modified_by *formatted_by,
+ struct apfs_modified_by *modified_by)
+{
+ struct apfs_modified_by *end_mod_by;
+ int length;
+ bool mods_over;
+ u64 xid;
+
+ mods_over = false;
+ end_mod_by = modified_by + APFS_MAX_HIST;
+ xid = sb->s_xid + 1; /* Last possible xid */
+
+ for (; modified_by != end_mod_by; ++modified_by) {
+ length = software_strlen(modified_by->id);
+ if (!length &&
+ (modified_by->timestamp || modified_by->last_xid))
+ report("Volume modification info", "entry without id.");
+
+ if (mods_over) {
+ if (length)
+ report("Volume modification info",
+ "empty entry should end the list.");
+ continue;
+ }
+ if (!length) {
+ mods_over = true;
+ continue;
+ }
+
+ /* The first entry must be the most recent */
+ if (xid <= le64_to_cpu(modified_by->last_xid))
+ report("Volume modification info",
+ "entries are not in order.");
+ xid = le64_to_cpu(modified_by->last_xid);
+ }
+
+ length = software_strlen(formatted_by->id);
+ if (!length)
+ report("Volume superblock", "creation information is missing.");
+ if (xid <= le64_to_cpu(formatted_by->last_xid))
+ report("Volume creation info", "transaction is too recent.");
+}
+
+/**
* map_volume_super - Find the volume superblock and map it into memory
* @vol: volume number
* @vsb: volume superblock struct to receive the results
@@ -163,6 +232,9 @@ static struct apfs_superblock *map_volume_super(int vol,
if (strnlen(vol_name, APFS_VOLNAME_LEN) == APFS_VOLNAME_LEN)
report("Volume superblock", "name lacks NULL-termination.");

+ check_software_information(&vsb->v_raw->apfs_formatted_by,
+ &vsb->v_raw->apfs_modified_by[0]);
+
if (le16_to_cpu(vsb->v_raw->reserved) != 0)
report("Volume superblock", "reserved field is in use.");

diff --git a/apfsck/super.h b/apfsck/super.h
index ab4db77..0c9f2fc 100644
--- a/apfsck/super.h
+++ b/apfsck/super.h
@@ -176,7 +176,7 @@ struct apfs_nx_superblock {
* Structure containing information about a program that modified the volume
*/
struct apfs_modified_by {
- char id[APFS_MODIFIED_NAMELEN];
+ u8 id[APFS_MODIFIED_NAMELEN];
__le64 timestamp;
__le64 last_xid;
} __packed;
--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:03:17 PM3/25/19
to linux...@googlegroups.com
The container superblock must be updated with each transaction, so its
xid must come right before the first available one. Add a check for
this.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index 0f4fb86..659e2de 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -125,6 +125,8 @@ static void map_main_super(void)
/* TODO: the latest checkpoint and block zero are somehow different? */
if (sb->s_xid != le64_to_cpu(msb_raw->nx_o.o_xid))
report_crash("Block zero");
+ if (sb->s_xid + 1 != le64_to_cpu(msb_raw->nx_next_xid))
+ report("Container superblock", "next transaction id is wrong.");
munmap(msb_raw, sb->s_blocksize);
}

--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:03:32 PM3/25/19
to linux...@googlegroups.com
Check that the transaction id of all objects inside a volume comes after
the first one, i.e., the one reported by apfs_formatted_by.

Note that it isn't true in general that the last transaction matches the
latest entry in apfs_modified_by, because keeping those updated is not
mandatory.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/object.c | 4 ++++
apfsck/super.c | 4 +++-
apfsck/super.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/apfsck/object.c b/apfsck/object.c
index 21d14d5..a8beaef 100644
--- a/apfsck/object.c
+++ b/apfsck/object.c
@@ -83,6 +83,10 @@ void *read_object(u64 oid, struct node *omap, struct object *obj)
if (!xid || sb->s_xid < xid)
report("Object header", "bad transaction id in block 0x%llx.",
(unsigned long long)bno);
+ if (vsb && vsb->v_first_xid > xid)
+ report("Object header",
+ "transaction id in block 0x%llx is older than volume.",
+ (unsigned long long)bno);
if (omap && xid != omap_rec.xid)
report("Object header",
"transaction id in omap key doesn't match block 0x%llx.",
diff --git a/apfsck/super.c b/apfsck/super.c
index 659e2de..b2671ad 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -195,7 +195,9 @@ static void check_software_information(struct apfs_modified_by *formatted_by,
length = software_strlen(formatted_by->id);
if (!length)
report("Volume superblock", "creation information is missing.");
- if (xid <= le64_to_cpu(formatted_by->last_xid))
+
+ vsb->v_first_xid = le64_to_cpu(formatted_by->last_xid);
+ if (xid <= vsb->v_first_xid)
report("Volume creation info", "transaction is too recent.");
}

diff --git a/apfsck/super.h b/apfsck/super.h
index 0c9f2fc..c7d1447 100644
--- a/apfsck/super.h
+++ b/apfsck/super.h
@@ -273,6 +273,7 @@ struct volume_superblock {
bool v_has_priv; /* Is there a private directory? */

/* Volume information read from the on-disk structure */
+ u64 v_first_xid; /* Transaction that created the volume */
u64 v_next_obj_id; /* Next cnid to be assigned */
u32 v_next_doc_id; /* Next document identifier to be assigned */

--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:03:43 PM3/25/19
to linux...@googlegroups.com
The specification is light on details about the purpose of the
apfs_root_to_xid field of the volume superblock. I believe it should
typically be set to 0; if it isn't, report it as unsupported.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index b2671ad..8502f86 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -241,6 +241,8 @@ static struct apfs_superblock *map_volume_super(int vol,

if (le16_to_cpu(vsb->v_raw->reserved) != 0)
report("Volume superblock", "reserved field is in use.");
+ if (le64_to_cpu(vsb->v_raw->apfs_root_to_xid) != 0)
+ report_unknown("Root from snapshot");

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:03:55 PM3/25/19
to linux...@googlegroups.com
Support for encryption is not planned for the near future. If a volume
is going through this process, report the feature as unknown.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index 8502f86..88820e8 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -243,6 +243,8 @@ static struct apfs_superblock *map_volume_super(int vol,
report("Volume superblock", "reserved field is in use.");
if (le64_to_cpu(vsb->v_raw->apfs_root_to_xid) != 0)
report_unknown("Root from snapshot");
+ if (le64_to_cpu(vsb->v_raw->apfs_er_state_oid) != 0)
+ report_unknown("Encryption or decryption in progress");

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:04:07 PM3/25/19
to linux...@googlegroups.com
Verify that a volume's flags are all valid and not reserved. Also
report the presence of any unsupported features.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index 88820e8..d25b2aa 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -202,6 +202,26 @@ static void check_software_information(struct apfs_modified_by *formatted_by,
}

/**
+ * check_volume_flags - Check consistency of volume flags
+ * @flags: the flags
+ */
+static void check_volume_flags(u64 flags)
+{
+ if ((flags & APFS_FS_FLAGS_VALID_MASK) != flags)
+ report("Volume superblock", "invalid flag in use.");
+ if (flags & APFS_FS_RESERVED_4)
+ report("Volume superblock", "reserved flag in use.");
+
+ if (!(flags & APFS_FS_UNENCRYPTED))
+ report_unknown("Encryption");
+ else if (flags & (APFS_FS_EFFACEABLE | APFS_FS_ONEKEY))
+ report("Volume superblock", "inconsistent crypto flags.");
+
+ if (flags & (APFS_FS_SPILLEDOVER | APFS_FS_RUN_SPILLOVER_CLEANER))
+ report_unknown("Fusion drive");
+}
+
+/**
* map_volume_super - Find the volume superblock and map it into memory
* @vol: volume number
* @vsb: volume superblock struct to receive the results
@@ -236,6 +256,7 @@ static struct apfs_superblock *map_volume_super(int vol,
if (strnlen(vol_name, APFS_VOLNAME_LEN) == APFS_VOLNAME_LEN)
report("Volume superblock", "name lacks NULL-termination.");

+ check_volume_flags(le64_to_cpu(vsb->v_raw->apfs_fs_flags));
check_software_information(&vsb->v_raw->apfs_formatted_by,
&vsb->v_raw->apfs_modified_by[0]);

--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:04:21 PM3/25/19
to linux...@googlegroups.com
Report as unsupported any volume that is in the process of reverting to
either a past snapshot or another superblock.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index d25b2aa..a4c1686 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -266,6 +266,10 @@ static struct apfs_superblock *map_volume_super(int vol,
report_unknown("Root from snapshot");
if (le64_to_cpu(vsb->v_raw->apfs_er_state_oid) != 0)
report_unknown("Encryption or decryption in progress");
+ if (le64_to_cpu(vsb->v_raw->apfs_revert_to_xid) != 0)
+ report_unknown("Revert to a snapshot");
+ if (le64_to_cpu(vsb->v_raw->apfs_revert_to_sblock_oid) != 0)
+ report_unknown("Revert to a volume superblock");

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:04:34 PM3/25/19
to linux...@googlegroups.com
The volume superblock reports the type of its catalog, extentref and
snapshot trees. Check that they match the "typical" values given by the
specification, since it's not clear to me how other types could ever be
used.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index a4c1686..1b10f21 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -260,6 +260,20 @@ static struct apfs_superblock *map_volume_super(int vol,
check_software_information(&vsb->v_raw->apfs_formatted_by,
&vsb->v_raw->apfs_modified_by[0]);

+ /*
+ * The documentation suggests that other tree types could be possible,
+ * but I don't understand how that would work.
+ */
+ if (le32_to_cpu(vsb->v_raw->apfs_root_tree_type) !=
+ (APFS_OBJ_VIRTUAL | APFS_OBJECT_TYPE_BTREE))
+ report("Volume superblock", "wrong type for catalog tree.");
+ if (le32_to_cpu(vsb->v_raw->apfs_extentref_tree_type) !=
+ (APFS_OBJ_PHYSICAL | APFS_OBJECT_TYPE_BTREE))
+ report("Volume superblock", "wrong type for extentref tree.");
+ if (le32_to_cpu(vsb->v_raw->apfs_snap_meta_tree_type) !=
+ (APFS_OBJ_PHYSICAL | APFS_OBJECT_TYPE_BTREE))
+ report("Volume superblock", "wrong type for snapshot tree.");
+
if (le16_to_cpu(vsb->v_raw->reserved) != 0)
report("Volume superblock", "reserved field is in use.");
if (le64_to_cpu(vsb->v_raw->apfs_root_to_xid) != 0)
--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:04:46 PM3/25/19
to linux...@googlegroups.com
Each volume superblock reports the volume's index in the container's
nx_fs_oid array. Check that it's correct.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index 1b10f21..b2868e0 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -246,6 +246,8 @@ static struct apfs_superblock *map_volume_super(int vol,
if (vsb->v_obj.subtype != APFS_OBJECT_TYPE_INVALID)
report("Volume superblock", "wrong object subtype.");

+ if (le32_to_cpu(vsb->v_raw->apfs_fs_index) != vol)
+ report("Volume superblock", "wrong reported volume number.");
if (le32_to_cpu(vsb->v_raw->apfs_magic) != APFS_MAGIC)
report("Volume superblock", "wrong magic.");

--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:05:07 PM3/25/19
to linux...@googlegroups.com
Check that the array of volume ids stored in the container superblock
does not have any more valid entries after its null termination.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/apfsck/super.c b/apfsck/super.c
index b2868e0..3e78055 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -237,8 +237,13 @@ static struct apfs_superblock *map_volume_super(int vol,
u64 vol_id;

vol_id = le64_to_cpu(msb_raw->nx_fs_oid[vol]);
- if (vol_id == 0)
+ if (vol_id == 0) {
+ for (++vol; vol < APFS_NX_MAX_FILE_SYSTEMS; ++vol)
+ if (msb_raw->nx_fs_oid[vol])
+ report("Container superblock",
+ "volume array goes on after NULL.");
return NULL;
+ }

vsb->v_raw = read_object(vol_id, sb->s_omap->root, &vsb->v_obj);
if (vsb->v_obj.type != APFS_OBJECT_TYPE_FS)
--
2.11.0

Ernesto A. Fernández

unread,
Mar 25, 2019, 3:05:24 PM3/25/19
to linux...@googlegroups.com
Check that the features reported by a volume superblock are all valid
and supported.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/super.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)

diff --git a/apfsck/super.c b/apfsck/super.c
index 3e78055..3420a4d 100644
--- a/apfsck/super.c
+++ b/apfsck/super.c
@@ -222,6 +222,54 @@ static void check_volume_flags(u64 flags)
}

/**
+ * check_optional_vol_features - Check the optional features of a volume
+ * @flags: the optional feature flags
+ */
+static void check_optional_vol_features(u64 flags)
+{
+ if ((flags & APFS_SUPPORTED_FEATURES_MASK) != flags)
+ report("Volume superblock", "unknown optional feature.");
+ if (flags & APFS_FEATURE_DEFRAG_PRERELEASE)
+ report("Volume superblock", "prerelease defrag enabled.");
+
+ /* TODO: should be easy to support, but I need an image for testing */
+ if (!(flags & APFS_FEATURE_HARDLINK_MAP_RECORDS))
+ report_unknown("Volume without sibling map records");
+}
+
+/**
+ * check_rocompat_vol_features - Check the ro compatible features of a volume
+ * @flags: the read-only compatible feature flags
+ */
+static void check_rocompat_vol_features(u64 flags)
+{
+ if ((flags & APFS_SUPPORTED_ROCOMPAT_MASK) != flags)
+ report("Volume superblock", "unknown ro compatible feature.");
+}
+
+/**
+ * check_incompat_vol_features - Check the incompatible features of a volume
+ * @flags: the incompatible feature flags
+ */
+static void check_incompat_vol_features(u64 flags)
+{
+ if ((flags & APFS_SUPPORTED_INCOMPAT_MASK) != flags)
+ report("Volume superblock", "unknown incompatible feature.");
+ if (flags & APFS_INCOMPAT_DATALESS_SNAPS)
+ report_unknown("Dataless snapshots");
+ if (flags & APFS_INCOMPAT_ENC_ROLLED)
+ report_unknown("Change of encryption keys");
+
+ /*
+ * I don't believe actual normalization-sensitive volumes exist, the
+ * normalization-insensitive flag just means case-sensitive.
+ */
+ if ((bool)(flags & APFS_INCOMPAT_CASE_INSENSITIVE) !=
+ !(bool)(flags & APFS_INCOMPAT_NORMALIZATION_INSENSITIVE))
+ report("Volume superblock", "normalization sensitive?");
+}
+
+/**
* map_volume_super - Find the volume superblock and map it into memory
* @vol: volume number
* @vsb: volume superblock struct to receive the results
@@ -256,6 +304,12 @@ static struct apfs_superblock *map_volume_super(int vol,
if (le32_to_cpu(vsb->v_raw->apfs_magic) != APFS_MAGIC)
report("Volume superblock", "wrong magic.");

+ check_optional_vol_features(le64_to_cpu(vsb->v_raw->apfs_features));
+ check_rocompat_vol_features(le64_to_cpu(
+ vsb->v_raw->apfs_readonly_compatible_features));
+ check_incompat_vol_features(le64_to_cpu(
+ vsb->v_raw->apfs_incompatible_features));
+
vsb->v_next_obj_id = le64_to_cpu(vsb->v_raw->apfs_next_obj_id);
vsb->v_next_doc_id = le32_to_cpu(vsb->v_raw->apfs_next_doc_id);

--
2.11.0

Reply all
Reply to author
Forward
0 new messages