| Message ID | 4c0d4b0003c31f1c1ee1d7a475f70f25663b98f6.1657919808.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-corrupt-block: btree data corruption | expand |
I thought I replied with this, but I can't find any evidence that the message actually sent, so: apologies if this is a duplication of a previous message... >> - printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the >> passed key triplet (must also specify the field to corrupt and root >> for the item)\n"); >> + printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the >> passed key triplet (must also specify the field, or bytes, offset, and >> value to corrupt and root for the item)\n"); > > I'd find it a little more understandable, even though I know it's not > intended to be read often, as: > + printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the > passed key triplet (must also specify the field, or (bytes, offset, > value) tuple, to corrupt, and root for the item)\n"); > > > >> + data = btrfs_item_ptr(leaf, slot, void); >> + // TODO: check offset/size legitimacy > > Is it worth fixing this todo? > > I'd prefer if there was a check that the existing data at the offset > isn't the same as the new value, so as to ensure we notice if we're > failing to corrupt. > >> + case 'v': >> + bogus_value = arg_strtou64(optarg); >> + break; > > You're parsing, and storing here, a u64; meanwhile > corrupt_btrfs_item_data() takes a char bogus_value. I think it > probably makes sense to only parse and store a char, but > >> + else if (bogus_offset != (u64)-1 && >> + bytes != (u64)-1 && >> + bogus_value != (u64)-1) > Might be worth calling out (u64)-1 as #define UNSET_VALUE for easier > readability? > >> + ret = corrupt_btrfs_item_data(target_root, &key, >> + bogus_offset, bytes, >> + bogus_value); >> + else >> + print_usage(1); > Maybe add an extra message here to say that either field or all of > offset, bytes, and value have to be set? > > Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
On 16.07.22 г. 0:22 ч., Boris Burkov wrote: > btrfs-corrupt-block already has a mix of generic and specific corruption > options, but currently lacks the capacity for totally arbitrary > corruption in item data. > > There is already a flag for corruption size (bytes/-b), so add a flag > for an offset and a value to memset the item with. Exercise the new > flags with a new variant for -I (item) corruption. Look up the item as > before, but instead of corrupting a field in the item struct, corrupt an > offset/size in the item data. > > The motivating example for this is that in testing fsverity with btrfs, > we need to corrupt the generated Merkle tree--metadata item data which > is an opaque blob to btrfs. > > Signed-off-by: Boris Burkov <boris@bur.io> > --- > btrfs-corrupt-block.c | 71 +++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 68 insertions(+), 3 deletions(-) > > diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c > index e961255d5..5c39459db 100644 > --- a/btrfs-corrupt-block.c > +++ b/btrfs-corrupt-block.c > @@ -98,12 +98,14 @@ static void print_usage(int ret) > printf("\t-m The metadata block to corrupt (must also specify -f for the field to corrupt)\n"); > printf("\t-K <u64,u8,u64> Corrupt the given key (must also specify -f for the field and optionally -r for the root)\n"); > printf("\t-f The field in the item to corrupt\n"); > - printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the passed key triplet (must also specify the field to corrupt and root for the item)\n"); > + printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the passed key triplet (must also specify the field, or bytes, offset, and value to corrupt and root for the item)\n"); > printf("\t-D <u64,u8,u64> Corrupt a dir item corresponding to the passed key triplet, must also specify a field\n"); > printf("\t-d <u64,u8,u64> Delete item corresponding to passed key triplet\n"); > printf("\t-r Operate on this root\n"); > printf("\t-C Delete a csum for the specified bytenr. When used with -b it'll delete that many bytes, otherwise it's just sectorsize\n"); > printf("\t--block-group OFFSET corrupt the given block group\n"); > + printf("\t-v Value to use for corrupting item data\n"); > + printf("\t-o Offset to use for corrupting item data\n"); > exit(ret); > } > > @@ -974,6 +976,50 @@ out: > return ret; > } > > +static int corrupt_btrfs_item_data(struct btrfs_root *root, > + struct btrfs_key *key, > + u64 bogus_offset, u64 bogus_size, > + char bogus_value) > +{ > + struct btrfs_trans_handle *trans; > + struct btrfs_path *path; > + int ret; > + void *data; > + struct extent_buffer *leaf; > + int slot; > + > + path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > + > + trans = btrfs_start_transaction(root, 1); > + if (IS_ERR(trans)) { > + fprintf(stderr, "Couldn't start transaction %ld\n", > + PTR_ERR(trans)); > + ret = PTR_ERR(trans); > + goto free_path; > + } > + > + ret = btrfs_search_slot(trans, root, key, path, 0, 1); > + if (ret != 0) { > + fprintf(stderr, "Error searching to node %d\n", ret); > + goto commit_txn; > + } > + leaf = path->nodes[0]; > + slot = path->slots[0]; > + data = btrfs_item_ptr(leaf, slot, void); > + // TODO: check offset/size legitimacy > + data += bogus_offset; > + memset_extent_buffer(leaf, bogus_value, (unsigned long)data, bogus_size); > + btrfs_mark_buffer_dirty(leaf); > + > +commit_txn: > + btrfs_commit_transaction(trans, root); > +free_path: > + btrfs_free_path(path); > + return ret; > +} > + > static int delete_item(struct btrfs_root *root, struct btrfs_key *key) > { > struct btrfs_trans_handle *trans; > @@ -1230,6 +1276,8 @@ int main(int argc, char **argv) > u64 csum_bytenr = 0; > u64 block_group = 0; > char field[FIELD_BUF_LEN]; > + u64 bogus_value = (u64)-1; > + u64 bogus_offset = (u64)-1; nit: Why the casts, according to http://port70.net/~nsz/c/c99/n1256.html#6.3.1.3 : 2 Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type.49) So in this case the correct thing would happen without the casts. <snip>
diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index e961255d5..5c39459db 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -98,12 +98,14 @@ static void print_usage(int ret) printf("\t-m The metadata block to corrupt (must also specify -f for the field to corrupt)\n"); printf("\t-K <u64,u8,u64> Corrupt the given key (must also specify -f for the field and optionally -r for the root)\n"); printf("\t-f The field in the item to corrupt\n"); - printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the passed key triplet (must also specify the field to corrupt and root for the item)\n"); + printf("\t-I <u64,u8,u64> Corrupt an item corresponding to the passed key triplet (must also specify the field, or bytes, offset, and value to corrupt and root for the item)\n"); printf("\t-D <u64,u8,u64> Corrupt a dir item corresponding to the passed key triplet, must also specify a field\n"); printf("\t-d <u64,u8,u64> Delete item corresponding to passed key triplet\n"); printf("\t-r Operate on this root\n"); printf("\t-C Delete a csum for the specified bytenr. When used with -b it'll delete that many bytes, otherwise it's just sectorsize\n"); printf("\t--block-group OFFSET corrupt the given block group\n"); + printf("\t-v Value to use for corrupting item data\n"); + printf("\t-o Offset to use for corrupting item data\n"); exit(ret); } @@ -974,6 +976,50 @@ out: return ret; } +static int corrupt_btrfs_item_data(struct btrfs_root *root, + struct btrfs_key *key, + u64 bogus_offset, u64 bogus_size, + char bogus_value) +{ + struct btrfs_trans_handle *trans; + struct btrfs_path *path; + int ret; + void *data; + struct extent_buffer *leaf; + int slot; + + path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; + + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + fprintf(stderr, "Couldn't start transaction %ld\n", + PTR_ERR(trans)); + ret = PTR_ERR(trans); + goto free_path; + } + + ret = btrfs_search_slot(trans, root, key, path, 0, 1); + if (ret != 0) { + fprintf(stderr, "Error searching to node %d\n", ret); + goto commit_txn; + } + leaf = path->nodes[0]; + slot = path->slots[0]; + data = btrfs_item_ptr(leaf, slot, void); + // TODO: check offset/size legitimacy + data += bogus_offset; + memset_extent_buffer(leaf, bogus_value, (unsigned long)data, bogus_size); + btrfs_mark_buffer_dirty(leaf); + +commit_txn: + btrfs_commit_transaction(trans, root); +free_path: + btrfs_free_path(path); + return ret; +} + static int delete_item(struct btrfs_root *root, struct btrfs_key *key) { struct btrfs_trans_handle *trans; @@ -1230,6 +1276,8 @@ int main(int argc, char **argv) u64 csum_bytenr = 0; u64 block_group = 0; char field[FIELD_BUF_LEN]; + u64 bogus_value = (u64)-1; + u64 bogus_offset = (u64)-1; field[0] = '\0'; memset(&key, 0, sizeof(key)); @@ -1258,11 +1306,13 @@ int main(int argc, char **argv) { "root", no_argument, NULL, 'r'}, { "csum", required_argument, NULL, 'C'}, { "block-group", required_argument, NULL, GETOPT_VAL_BLOCK_GROUP}, + { "value", required_argument, NULL, 'v'}, + { "offset", required_argument, NULL, 'o'}, { "help", no_argument, NULL, GETOPT_VAL_HELP}, { NULL, 0, NULL, 0 } }; - c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:I:D:d:r:C:", + c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:I:D:d:r:C:v:o:", long_options, NULL); if (c < 0) break; @@ -1328,6 +1378,12 @@ int main(int argc, char **argv) case GETOPT_VAL_BLOCK_GROUP: block_group = arg_strtou64(optarg); break; + case 'v': + bogus_value = arg_strtou64(optarg); + break; + case 'o': + bogus_offset = arg_strtou64(optarg); + break; case GETOPT_VAL_HELP: default: print_usage(c != GETOPT_VAL_HELP); @@ -1454,7 +1510,16 @@ int main(int argc, char **argv) if (!root_objectid) print_usage(1); - ret = corrupt_btrfs_item(target_root, &key, field); + if (*field != 0) + ret = corrupt_btrfs_item(target_root, &key, field); + else if (bogus_offset != (u64)-1 && + bytes != (u64)-1 && + bogus_value != (u64)-1) + ret = corrupt_btrfs_item_data(target_root, &key, + bogus_offset, bytes, + bogus_value); + else + print_usage(1); goto out_close; } if (delete) {
btrfs-corrupt-block already has a mix of generic and specific corruption options, but currently lacks the capacity for totally arbitrary corruption in item data. There is already a flag for corruption size (bytes/-b), so add a flag for an offset and a value to memset the item with. Exercise the new flags with a new variant for -I (item) corruption. Look up the item as before, but instead of corrupting a field in the item struct, corrupt an offset/size in the item data. The motivating example for this is that in testing fsverity with btrfs, we need to corrupt the generated Merkle tree--metadata item data which is an opaque blob to btrfs. Signed-off-by: Boris Burkov <boris@bur.io> --- btrfs-corrupt-block.c | 71 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-)