From 2fc1b99698abb0765386ec44aa17966ced65909a Mon Sep 17 00:00:00 2001 From: Mark Fasheh Date: Thu, 27 Oct 2016 14:24:40 -0500 Subject: [PATCH] scoutfs: replace some open coded corruption checks We can trivially do the simple check of value length against what the caller expects in btree.c. Signed-off-by: Mark Fasheh Signed-off-by: Zach Brown --- kmod/src/btree.c | 10 ++++++++++ kmod/src/btree.h | 1 + kmod/src/dir.c | 14 ++------------ kmod/src/filerw.c | 14 ++------------ kmod/src/inode.c | 14 ++------------ kmod/src/xattr.c | 14 ++------------ 6 files changed, 19 insertions(+), 48 deletions(-) diff --git a/kmod/src/btree.c b/kmod/src/btree.c index 441fb36b..0ea2d590 100644 --- a/kmod/src/btree.c +++ b/kmod/src/btree.c @@ -138,6 +138,16 @@ static int copy_to_val(struct scoutfs_btree_val *val, size_t off; int i; + /* + * Corruption check, right now we just return -EIO if the + * caller wants this. In the future we can grow this to do + * different things (go readonly, ignore, return error) based + * on the severity of the problem. + */ + /* XXX corruption */ + if (val->check_size_eq && val_len != scoutfs_btree_val_length(val)) + return -EIO; + for (i = 0, off = 0; val_len > 0 && i < ARRAY_SIZE(val->vec); i++) { kv = &val->vec[i]; diff --git a/kmod/src/btree.h b/kmod/src/btree.h index dc22b4c6..792ba83d 100644 --- a/kmod/src/btree.h +++ b/kmod/src/btree.h @@ -5,6 +5,7 @@ struct scoutfs_btree_val { struct kvec vec[3]; + unsigned int check_size_eq:1; }; static inline void __scoutfs_btree_init_val(struct scoutfs_btree_val *val, diff --git a/kmod/src/dir.c b/kmod/src/dir.c index d45fd05e..8a03f986 100644 --- a/kmod/src/dir.c +++ b/kmod/src/dir.c @@ -654,6 +654,7 @@ static void *scoutfs_follow_link(struct dentry *dentry, struct nameidata *nd) SCOUTFS_SYMLINK_KEY, k); bytes = min_t(int, size - off, SCOUTFS_MAX_ITEM_LEN); scoutfs_btree_init_val(&val, path + off, bytes); + val.check_size_eq = 1; ret = scoutfs_btree_lookup(sb, meta, &key, &val); if (ret < 0) { @@ -663,12 +664,6 @@ static void *scoutfs_follow_link(struct dentry *dentry, struct nameidata *nd) break; } - /* XXX corruption */ - if (ret != bytes) { - ret = -EIO; - break; - } - off += bytes; ret = 0; } @@ -858,6 +853,7 @@ retry: scoutfs_set_key(&last, ino, SCOUTFS_LINK_BACKREF_KEY, ~0ULL); scoutfs_btree_init_val(&val, &lref, sizeof(lref)); + val.check_size_eq = 1; ret = scoutfs_btree_next(sb, meta, &first, &last, &key, &val); if (ret < 0) { @@ -866,12 +862,6 @@ retry: goto out; } - /* XXX corruption */ - if (ret != sizeof(lref)) { - ret = -EIO; - goto out; - } - *dir_ino = le64_to_cpu(lref.ino), off = le64_to_cpu(lref.offset); *ctr = scoutfs_key_offset(&key); diff --git a/kmod/src/filerw.c b/kmod/src/filerw.c index 4cbac7b3..40429532 100644 --- a/kmod/src/filerw.c +++ b/kmod/src/filerw.c @@ -223,6 +223,7 @@ int scoutfs_truncate_block_items(struct super_block *sb, u64 ino, u64 size) trace_printk("iblock %llu i %d\n", iblock, i); scoutfs_btree_init_val(&val, &bmap, sizeof(bmap)); + val.check_size_eq = 1; for (;;) { ret = scoutfs_btree_next(sb, meta, &key, &last, &key, &val); @@ -232,12 +233,6 @@ int scoutfs_truncate_block_items(struct super_block *sb, u64 ino, u64 size) break; } - /* XXX corruption */ - if (ret != sizeof(bmap)) { - ret = -EIO; - break; - } - /* XXX check bmap sanity */ /* make sure we can update bmap after freeing */ @@ -374,6 +369,7 @@ static int map_writable_block(struct inode *inode, u64 iblock, u64 *blkno_ret) set_bmap_key(&key, inode, iblock); scoutfs_btree_init_val(&val, &bmap, sizeof(bmap)); + val.check_size_eq = 1; /* see if there's an existing mapping */ ret = scoutfs_btree_lookup(sb, meta, &key, &val); @@ -389,12 +385,6 @@ static int map_writable_block(struct inode *inode, u64 iblock, u64 *blkno_ret) inserted = true; } else { - /* XXX corruption */ - if (ret != sizeof(bmap)) { - ret = -EIO; - goto out; - } - ret = scoutfs_btree_dirty(sb, meta, &key); if (ret) goto out; diff --git a/kmod/src/inode.c b/kmod/src/inode.c index f8e6b6f8..da3e150f 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -427,16 +427,12 @@ static void delete_inode(struct super_block *sb, u64 ino) /* sample the inode mode, XXX don't need to copy whole thing here */ scoutfs_set_key(&key, ino, SCOUTFS_INODE_KEY, 0); scoutfs_btree_init_val(&val, &sinode, sizeof(sinode)); + val.check_size_eq = 1; ret = scoutfs_btree_lookup(sb, meta, &key, &val); if (ret < 0) goto out; - /* XXX corruption */ - if (ret != sizeof(sinode)) { - ret = -EIO; - goto out; - } mode = le32_to_cpu(sinode.mode); ret = __delete_inode(sb, &key, ino, mode); @@ -486,6 +482,7 @@ static int process_orphaned_inode(struct super_block *sb, u64 ino) scoutfs_set_key(&key, ino, SCOUTFS_INODE_KEY, 0); scoutfs_btree_init_val(&val, &sinode, sizeof(sinode)); + val.check_size_eq = 1; ret = scoutfs_btree_lookup(sb, meta, &key, &val); if (ret < 0) { @@ -494,18 +491,11 @@ static int process_orphaned_inode(struct super_block *sb, u64 ino) return ret; } - /* XXX corruption */ - if (ret != sizeof(sinode)) { - ret = -EIO; - goto out; - } - if (le32_to_cpu(sinode.nlink) == 0) __delete_inode(sb, &key, ino, le32_to_cpu(sinode.mode)); else scoutfs_warn(sb, "Dangling orphan item for inode %llu.", ino); -out: return ret; } diff --git a/kmod/src/xattr.c b/kmod/src/xattr.c index abfc6a8c..4a0a2a7c 100644 --- a/kmod/src/xattr.c +++ b/kmod/src/xattr.c @@ -230,6 +230,7 @@ static int insert_xattr(struct inode *inode, const char *name, /* increment the val hash item for find_xattr, inserting if first */ scoutfs_btree_init_val(&val, &refcount, sizeof(refcount)); + val.check_size_eq = 1; ret = scoutfs_btree_lookup(sb, meta, &val_key, &val); if (ret < 0 && ret != -ENOENT) @@ -239,12 +240,6 @@ static int insert_xattr(struct inode *inode, const char *name, refcount = cpu_to_le64(1); ret = scoutfs_btree_insert(sb, meta, &val_key, &val); } else { - /* XXX corruption */ - if (ret != sizeof(refcount)) { - ret = -EIO; - goto out; - } - le64_add_cpu(&refcount, 1); ret = scoutfs_btree_update(sb, meta, &val_key, &val); } @@ -277,16 +272,11 @@ static int delete_xattr(struct super_block *sb, struct scoutfs_key *key, /* update the val_hash refcount, making sure it's not nonsense */ scoutfs_btree_init_val(&val, &refcount, sizeof(refcount)); + val.check_size_eq = 1; ret = scoutfs_btree_lookup(sb, meta, &val_key, &val); if (ret < 0) goto out; - /* XXX corruption */ - if (ret != sizeof(refcount)) { - ret = -EIO; - goto out; - } - le64_add_cpu(&refcount, -1ULL); /* ensure that we can update and delete name_ and val_ keys */