[PATCH 1/2] apfsck: move inode size information to dstream

4 views
Skip to first unread message

Ernesto A. Fernández

unread,
Mar 14, 2019, 7:52:43 PM3/14/19
to linux...@googlegroups.com
Move the i_size and i_alloced_size fields from the inode structure to
the associated dstream. This makes more sense because several different
inodes can share the same dstream, and xattrs can use them as well.

Check that every inode with the same private id reports the same size
for the dstream, and that no dstream is used by both an inode and a
xattr.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/extents.c | 22 ++++++++++++++++++++--
apfsck/extents.h | 9 ++++++++-
apfsck/inode.c | 35 +++++++++++++++++++++++++----------
apfsck/inode.h | 2 --
4 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/apfsck/extents.c b/apfsck/extents.c
index 32fbbbc..d8b441c 100644
--- a/apfsck/extents.c
+++ b/apfsck/extents.c
@@ -27,6 +27,22 @@ struct dstream **alloc_dstream_table(void)
}

/**
+ * check_dstream_stats - Verify the stats gathered by the fsck vs the metadata
+ * @dstream: dstream structure to check
+ */
+static void check_dstream_stats(struct dstream *dstream)
+{
+ if (dstream->d_obj_type != APFS_TYPE_INODE)
+ /* No checks for xattrs, for now */
+ return;
+
+ if (dstream->d_bytes < dstream->d_size)
+ report("Data stream", "some extents are missing.");
+ if (dstream->d_bytes != dstream->d_alloced_size)
+ report("Data stream", "wrong allocated space.");
+}
+
+/**
* free_dstream_table - Free the dstream hash table and all its dstreams
* @table: table to free
*/
@@ -39,6 +55,8 @@ void free_dstream_table(struct dstream **table)
for (i = 0; i < DSTREAM_TABLE_BUCKETS; ++i) {
current = table[i];
while (current) {
+ check_dstream_stats(current);
+
next = current->d_next;
free(current);
current = next;
@@ -110,9 +128,9 @@ void parse_extent_record(struct apfs_file_extent_key *key,
report("Extent record", "no flags should be set.");

dstream = get_dstream(cat_cnid(&key->hdr), vsb->v_dstream_table);
- if (dstream->d_size != le64_to_cpu(key->logical_addr))
+ if (dstream->d_bytes != le64_to_cpu(key->logical_addr))
report("Data stream", "extents are not consecutive.");
- dstream->d_size += length;
+ dstream->d_bytes += length;

if (!le64_to_cpu(val->phys_block_num)) /* This is a hole */
dstream->d_sparse_bytes += length;
diff --git a/apfsck/extents.h b/apfsck/extents.h
index d774e54..db11913 100644
--- a/apfsck/extents.h
+++ b/apfsck/extents.h
@@ -32,7 +32,14 @@ struct apfs_file_extent_val {
*/
struct dstream {
u64 d_id; /* Id of the dstream */
- u64 d_size; /* Size of the extents read so far */
+ u8 d_obj_type; /* Type of the owner objects */
+
+ /* Dstream stats read from the dstream structure */
+ u64 d_size; /* Dstream size */
+ u64 d_alloced_size; /* Dstream size, including unused */
+
+ /* Dstream stats measured by the fsck */
+ u64 d_bytes; /* Size of the extents read so far */
u64 d_sparse_bytes; /* Size of the holes read so far */

struct dstream *d_next; /* Next dstream in linked list */
diff --git a/apfsck/inode.c b/apfsck/inode.c
index a5cfbac..0c0d060 100644
--- a/apfsck/inode.c
+++ b/apfsck/inode.c
@@ -37,10 +37,6 @@ static void check_inode_stats(struct inode *inode)
}

dstream = get_dstream(inode->i_private_id, vsb->v_dstream_table);
- if (dstream->d_size < inode->i_size)
- report("Inode record", "some extents are missing.");
- if (dstream->d_size != inode->i_alloced_size)
- report("Inode record", "wrong allocated space for dstream.");
if (dstream->d_sparse_bytes != inode->i_sparse_bytes)
report("Inode record", "wrong count of sparse bytes.");

@@ -300,16 +296,35 @@ static int read_name_xfield(char *xval, int len, struct inode *inode)
*/
static int read_dstream_xfield(char *xval, int len, struct inode *inode)
{
- struct apfs_dstream *dstream;
+ struct apfs_dstream *dstream_raw;
+ struct dstream *dstream;
+ u64 size, alloced_size;

- if (len < sizeof(*dstream))
+ if (len < sizeof(*dstream_raw))
report("Dstream xfield", "doesn't fit in inode record.");
- dstream = (struct apfs_dstream *)xval;
+ dstream_raw = (struct apfs_dstream *)xval;
+
+ size = le64_to_cpu(dstream_raw->size);
+ alloced_size = le64_to_cpu(dstream_raw->alloced_size);

- inode->i_size = le64_to_cpu(dstream->size);
- inode->i_alloced_size = le64_to_cpu(dstream->alloced_size);
+ dstream = get_dstream(inode->i_private_id, vsb->v_dstream_table);
+ if (dstream->d_obj_type) {
+ /* A dstream structure for this id has already been seen */
+ if (dstream->d_obj_type != APFS_TYPE_INODE)
+ report("Dstream xfield", "shared by inode and xattr.");
+ if (dstream->d_size != size)
+ report("Dstream xfield",
+ "inconsistent size for stream.");
+ if (dstream->d_alloced_size != alloced_size)
+ report("Dstream xfield",
+ "inconsistent allocated size for stream.");
+ } else {
+ dstream->d_obj_type = APFS_TYPE_INODE;
+ dstream->d_size = size;
+ dstream->d_alloced_size = alloced_size;
+ }

- return sizeof(*dstream);
+ return sizeof(*dstream_raw);
}

/**
diff --git a/apfsck/inode.h b/apfsck/inode.h
index c78c36e..292c5ed 100644
--- a/apfsck/inode.h
+++ b/apfsck/inode.h
@@ -196,8 +196,6 @@ struct inode {
u32 i_nchildren; /* Number of children of directory */
u32 i_nlink; /* Number of hard links to file */
};
- u64 i_size; /* Inode size */
- u64 i_alloced_size; /* Inode size, including unused */
u64 i_sparse_bytes; /* Number of sparse bytes */
u64 i_flags; /* Internal flags */
u32 i_rdev; /* Device ID */
--
2.11.0

Ernesto A. Fernández

unread,
Mar 14, 2019, 7:52:55 PM3/14/19
to linux...@googlegroups.com
Some xattrs keep their contents in a data stream, like inodes do, so run
the same checks on them.

Many file extent records still appear to be orphaned, so it's likely
that other filesystem objects have streams as well. Until I figure it
out, report this as unsupported.

Signed-off-by: Ernesto A. Fernández <ernesto.mn...@gmail.com>
---
apfsck/extents.c | 6 ++----
apfsck/xattr.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/apfsck/extents.c b/apfsck/extents.c
index d8b441c..b3f52e2 100644
--- a/apfsck/extents.c
+++ b/apfsck/extents.c
@@ -32,10 +32,8 @@ struct dstream **alloc_dstream_table(void)
*/
static void check_dstream_stats(struct dstream *dstream)
{
- if (dstream->d_obj_type != APFS_TYPE_INODE)
- /* No checks for xattrs, for now */
- return;
-
+ if (!dstream->d_obj_type) /* The dstream structure was never seen */
+ report_unknown("File extent without inode or xattr");
if (dstream->d_bytes < dstream->d_size)
report("Data stream", "some extents are missing.");
if (dstream->d_bytes != dstream->d_alloced_size)
diff --git a/apfsck/xattr.c b/apfsck/xattr.c
index f509ef3..07bb207 100644
--- a/apfsck/xattr.c
+++ b/apfsck/xattr.c
@@ -6,6 +6,7 @@

#include <string.h>
#include "apfsck.h"
+#include "extents.h"
#include "inode.h"
#include "key.h"
#include "super.h"
@@ -29,6 +30,38 @@ static void check_xattr_flags(u16 flags)
}

/**
+ * parse_xattr_dstream - Parse a xattr dstream struct and check for corruption
+ * @xstream: the dstream structure
+ */
+static void parse_xattr_dstream(struct apfs_xattr_dstream *xstream)
+{
+ struct dstream *dstream;
+ struct apfs_dstream *dstream_raw = &xstream->dstream;
+ u64 id = le64_to_cpu(xstream->xattr_obj_id);
+ u64 size, alloced_size;
+
+ size = le64_to_cpu(dstream_raw->size);
+ alloced_size = le64_to_cpu(dstream_raw->alloced_size);
+
+ dstream = get_dstream(id, vsb->v_dstream_table);
+ if (dstream->d_obj_type) {
+ /* A dstream structure for this id has already been seen */
+ if (dstream->d_obj_type != APFS_TYPE_XATTR)
+ report("Xattr dstream", "shared by inode and xattr.");
+ if (dstream->d_size != size)
+ report("Xattr dstream",
+ "inconsistent size for stream.");
+ if (dstream->d_alloced_size != alloced_size)
+ report("Xattr dstream",
+ "inconsistent allocated size for stream.");
+ } else {
+ dstream->d_obj_type = APFS_TYPE_XATTR;
+ dstream->d_size = size;
+ dstream->d_alloced_size = alloced_size;
+ }
+}
+
+/**
* parse_xattr_record - Parse a xattr record value and check for corruption
* @key: pointer to the raw key
* @val: pointer to the raw value
@@ -56,6 +89,7 @@ void parse_xattr_record(struct apfs_xattr_key *key,
if (len != le16_to_cpu(val->xdata_len))
/* Never seems to happen, but the docs don't ban it */
report_weird("Xattr data length for dstream structure");
+ parse_xattr_dstream((struct apfs_xattr_dstream *)val->xdata);
} else {
if (len != le16_to_cpu(val->xdata_len))
report("Xattr record", "bad length for embedded data.");
--
2.11.0

Reply all
Reply to author
Forward
0 new messages