Message ID | 20200809120919.85271-4-wqu@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | btrfs: Enhanced runtime defence against fuzzed images | expand |
On 8/9/20 8:09 AM, Qu Wenruo wrote: > [BUG] > With crafted image, btrfs will panic at btree operations: > kernel BUG at fs/btrfs/ctree.c:3894! > invalid opcode: 0000 [#1] SMP PTI > CPU: 0 PID: 1138 Comm: btrfs-transacti Not tainted 5.0.0-rc8+ #9 > RIP: 0010:__push_leaf_left+0x6b6/0x6e0 > Code: 00 00 48 98 48 8d 04 80 48 8d 74 80 65 e8 42 5a 04 00 48 8b bd 78 ff ff ff 8b bf 90 d0 00 00 89 7d 98 83 ef 65 e9 06 ff ff ff <0f> 0b 0f 0b 48 8b 85 78 ff ff ff 8b 90 90 d0 00 00 e9 eb fe ff ff > RSP: 0018:ffffc0bd4128b990 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: ffffa0a4ab8f0e38 RCX: 0000000000000000 > RDX: ffffa0a280000000 RSI: 0000000000000000 RDI: ffffa0a4b3814000 > RBP: ffffc0bd4128ba38 R08: 0000000000001000 R09: ffffc0bd4128b948 > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000240 > R13: ffffa0a4b556fb60 R14: ffffa0a4ab8f0af0 R15: ffffa0a4ab8f0af0 > FS: 0000000000000000(0000) GS:ffffa0a4b7a00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f2461c80020 CR3: 000000022b32a006 CR4: 00000000000206f0 > Call Trace: > ? _cond_resched+0x1a/0x50 > push_leaf_left+0x179/0x190 > btrfs_del_items+0x316/0x470 > btrfs_del_csums+0x215/0x3a0 > __btrfs_free_extent.isra.72+0x5a7/0xbe0 > __btrfs_run_delayed_refs+0x539/0x1120 > btrfs_run_delayed_refs+0xdb/0x1b0 > btrfs_commit_transaction+0x52/0x950 > ? start_transaction+0x94/0x450 > transaction_kthread+0x163/0x190 > kthread+0x105/0x140 > ? btrfs_cleanup_transaction+0x560/0x560 > ? kthread_destroy_worker+0x50/0x50 > ret_from_fork+0x35/0x40 > Modules linked in: > ---[ end trace c2425e6e89b5558f ]--- > > [CAUSE] > The offending csum tree looks like this: > checksum tree key (CSUM_TREE ROOT_ITEM 0) > node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE > ... > key (EXTENT_CSUM EXTENT_CSUM 85975040) block 29630464 gen 17 > key (EXTENT_CSUM EXTENT_CSUM 89911296) block 29642752 gen 17 <<< > key (EXTENT_CSUM EXTENT_CSUM 92274688) block 29646848 gen 17 > ... > > leaf 29630464 items 6 free space 1 generation 17 owner CSUM_TREE > item 0 key (EXTENT_CSUM EXTENT_CSUM 85975040) itemoff 3987 itemsize 8 > range start 85975040 end 85983232 length 8192 > ... > leaf 29642752 items 0 free space 3995 generation 17 owner 0 > ^ empty leaf invalid owner ^ > > leaf 29646848 items 1 free space 602 generation 17 owner CSUM_TREE > item 0 key (EXTENT_CSUM EXTENT_CSUM 92274688) itemoff 627 itemsize 3368 > range start 92274688 end 95723520 length 3448832 > > So we have a corrupted csum tree where one tree leaf is completely > empty, causing unbalanced btree, thus leading to unexpected btree > balance error. > > [FIX] > For this particular case, we handle it in two directions to catch it: > - Check if the tree block is empty through btrfs_verify_level_key() > So that invalid tree blocks won't be read out through > btrfs_search_slot() and its variants. > > - Check 0 tree owner in tree checker > NO tree is using 0 as its tree owner, detect it and reject at tree > block read time. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202821 > Signed-off-by: Qu Wenruo <wqu@suse.com> This test is done further down, just after a if (btrfs_header_generation(eb) > fs_info->last_trans_committed) return 0; Which I assume is the problem? The generation is 19, is that > last_trans_committed? Seems like this check just needs to be moved lower, right? Thanks, Josef
On 2020/8/12 上午2:48, Josef Bacik wrote: > On 8/9/20 8:09 AM, Qu Wenruo wrote: >> [BUG] >> With crafted image, btrfs will panic at btree operations: >> kernel BUG at fs/btrfs/ctree.c:3894! >> invalid opcode: 0000 [#1] SMP PTI >> CPU: 0 PID: 1138 Comm: btrfs-transacti Not tainted 5.0.0-rc8+ #9 >> RIP: 0010:__push_leaf_left+0x6b6/0x6e0 >> Code: 00 00 48 98 48 8d 04 80 48 8d 74 80 65 e8 42 5a 04 00 48 8b >> bd 78 ff ff ff 8b bf 90 d0 00 00 89 7d 98 83 ef 65 e9 06 ff ff ff <0f> >> 0b 0f 0b 48 8b 85 78 ff ff ff 8b 90 90 d0 00 00 e9 eb fe ff ff >> RSP: 0018:ffffc0bd4128b990 EFLAGS: 00010246 >> RAX: 0000000000000000 RBX: ffffa0a4ab8f0e38 RCX: 0000000000000000 >> RDX: ffffa0a280000000 RSI: 0000000000000000 RDI: ffffa0a4b3814000 >> RBP: ffffc0bd4128ba38 R08: 0000000000001000 R09: ffffc0bd4128b948 >> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000240 >> R13: ffffa0a4b556fb60 R14: ffffa0a4ab8f0af0 R15: ffffa0a4ab8f0af0 >> FS: 0000000000000000(0000) GS:ffffa0a4b7a00000(0000) >> knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00007f2461c80020 CR3: 000000022b32a006 CR4: 00000000000206f0 >> Call Trace: >> ? _cond_resched+0x1a/0x50 >> push_leaf_left+0x179/0x190 >> btrfs_del_items+0x316/0x470 >> btrfs_del_csums+0x215/0x3a0 >> __btrfs_free_extent.isra.72+0x5a7/0xbe0 >> __btrfs_run_delayed_refs+0x539/0x1120 >> btrfs_run_delayed_refs+0xdb/0x1b0 >> btrfs_commit_transaction+0x52/0x950 >> ? start_transaction+0x94/0x450 >> transaction_kthread+0x163/0x190 >> kthread+0x105/0x140 >> ? btrfs_cleanup_transaction+0x560/0x560 >> ? kthread_destroy_worker+0x50/0x50 >> ret_from_fork+0x35/0x40 >> Modules linked in: >> ---[ end trace c2425e6e89b5558f ]--- >> >> [CAUSE] >> The offending csum tree looks like this: >> checksum tree key (CSUM_TREE ROOT_ITEM 0) >> node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE >> ... >> key (EXTENT_CSUM EXTENT_CSUM 85975040) block 29630464 gen 17 >> key (EXTENT_CSUM EXTENT_CSUM 89911296) block 29642752 gen 17 <<< >> key (EXTENT_CSUM EXTENT_CSUM 92274688) block 29646848 gen 17 >> ... >> >> leaf 29630464 items 6 free space 1 generation 17 owner CSUM_TREE >> item 0 key (EXTENT_CSUM EXTENT_CSUM 85975040) itemoff 3987 >> itemsize 8 >> range start 85975040 end 85983232 length 8192 >> ... >> leaf 29642752 items 0 free space 3995 generation 17 owner 0 >> ^ empty leaf invalid owner ^ >> >> leaf 29646848 items 1 free space 602 generation 17 owner CSUM_TREE >> item 0 key (EXTENT_CSUM EXTENT_CSUM 92274688) itemoff 627 >> itemsize 3368 >> range start 92274688 end 95723520 length 3448832 >> >> So we have a corrupted csum tree where one tree leaf is completely >> empty, causing unbalanced btree, thus leading to unexpected btree >> balance error. >> >> [FIX] >> For this particular case, we handle it in two directions to catch it: >> - Check if the tree block is empty through btrfs_verify_level_key() >> So that invalid tree blocks won't be read out through >> btrfs_search_slot() and its variants. >> >> - Check 0 tree owner in tree checker >> NO tree is using 0 as its tree owner, detect it and reject at tree >> block read time. >> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202821 >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > This test is done further down, just after a > > if (btrfs_header_generation(eb) > fs_info->last_trans_committed) > return 0; > > Which I assume is the problem? The generation is 19, is that > > last_trans_committed? Seems like this check just needs to be moved > lower, right? Thanks, Nope, that generation 19 is valid. That fs has a higher generation, so that's completely valid. The generation 19 is there because there is another csum leaf whose generation is 19. Thanks, Qu > > Josef
On 8/11/20 7:04 PM, Qu Wenruo wrote: > > > On 2020/8/12 上午2:48, Josef Bacik wrote: >> On 8/9/20 8:09 AM, Qu Wenruo wrote: >>> [BUG] >>> With crafted image, btrfs will panic at btree operations: >>> kernel BUG at fs/btrfs/ctree.c:3894! >>> invalid opcode: 0000 [#1] SMP PTI >>> CPU: 0 PID: 1138 Comm: btrfs-transacti Not tainted 5.0.0-rc8+ #9 >>> RIP: 0010:__push_leaf_left+0x6b6/0x6e0 >>> Code: 00 00 48 98 48 8d 04 80 48 8d 74 80 65 e8 42 5a 04 00 48 8b >>> bd 78 ff ff ff 8b bf 90 d0 00 00 89 7d 98 83 ef 65 e9 06 ff ff ff <0f> >>> 0b 0f 0b 48 8b 85 78 ff ff ff 8b 90 90 d0 00 00 e9 eb fe ff ff >>> RSP: 0018:ffffc0bd4128b990 EFLAGS: 00010246 >>> RAX: 0000000000000000 RBX: ffffa0a4ab8f0e38 RCX: 0000000000000000 >>> RDX: ffffa0a280000000 RSI: 0000000000000000 RDI: ffffa0a4b3814000 >>> RBP: ffffc0bd4128ba38 R08: 0000000000001000 R09: ffffc0bd4128b948 >>> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000240 >>> R13: ffffa0a4b556fb60 R14: ffffa0a4ab8f0af0 R15: ffffa0a4ab8f0af0 >>> FS: 0000000000000000(0000) GS:ffffa0a4b7a00000(0000) >>> knlGS:0000000000000000 >>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> CR2: 00007f2461c80020 CR3: 000000022b32a006 CR4: 00000000000206f0 >>> Call Trace: >>> ? _cond_resched+0x1a/0x50 >>> push_leaf_left+0x179/0x190 >>> btrfs_del_items+0x316/0x470 >>> btrfs_del_csums+0x215/0x3a0 >>> __btrfs_free_extent.isra.72+0x5a7/0xbe0 >>> __btrfs_run_delayed_refs+0x539/0x1120 >>> btrfs_run_delayed_refs+0xdb/0x1b0 >>> btrfs_commit_transaction+0x52/0x950 >>> ? start_transaction+0x94/0x450 >>> transaction_kthread+0x163/0x190 >>> kthread+0x105/0x140 >>> ? btrfs_cleanup_transaction+0x560/0x560 >>> ? kthread_destroy_worker+0x50/0x50 >>> ret_from_fork+0x35/0x40 >>> Modules linked in: >>> ---[ end trace c2425e6e89b5558f ]--- >>> >>> [CAUSE] >>> The offending csum tree looks like this: >>> checksum tree key (CSUM_TREE ROOT_ITEM 0) >>> node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE >>> ... >>> key (EXTENT_CSUM EXTENT_CSUM 85975040) block 29630464 gen 17 >>> key (EXTENT_CSUM EXTENT_CSUM 89911296) block 29642752 gen 17 <<< >>> key (EXTENT_CSUM EXTENT_CSUM 92274688) block 29646848 gen 17 >>> ... >>> >>> leaf 29630464 items 6 free space 1 generation 17 owner CSUM_TREE >>> item 0 key (EXTENT_CSUM EXTENT_CSUM 85975040) itemoff 3987 >>> itemsize 8 >>> range start 85975040 end 85983232 length 8192 >>> ... >>> leaf 29642752 items 0 free space 3995 generation 17 owner 0 >>> ^ empty leaf invalid owner ^ >>> >>> leaf 29646848 items 1 free space 602 generation 17 owner CSUM_TREE >>> item 0 key (EXTENT_CSUM EXTENT_CSUM 92274688) itemoff 627 >>> itemsize 3368 >>> range start 92274688 end 95723520 length 3448832 >>> >>> So we have a corrupted csum tree where one tree leaf is completely >>> empty, causing unbalanced btree, thus leading to unexpected btree >>> balance error. >>> >>> [FIX] >>> For this particular case, we handle it in two directions to catch it: >>> - Check if the tree block is empty through btrfs_verify_level_key() >>> So that invalid tree blocks won't be read out through >>> btrfs_search_slot() and its variants. >>> >>> - Check 0 tree owner in tree checker >>> NO tree is using 0 as its tree owner, detect it and reject at tree >>> block read time. >>> >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202821 >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >> >> This test is done further down, just after a >> >> if (btrfs_header_generation(eb) > fs_info->last_trans_committed) >> return 0; >> >> Which I assume is the problem? The generation is 19, is that > >> last_trans_committed? Seems like this check just needs to be moved >> lower, right? Thanks, > > Nope, that generation 19 is valid. That fs has a higher generation, so > that's completely valid. > > The generation 19 is there because there is another csum leaf whose > generation is 19. > Then this patch does nothing, because we already have this check lower, so how exactly did it make the panic go away? Thanks, Josef
On 2020/8/12 上午8:23, Josef Bacik wrote: > On 8/11/20 7:04 PM, Qu Wenruo wrote: >> >> [...] >>> Which I assume is the problem? The generation is 19, is that > >>> last_trans_committed? Seems like this check just needs to be moved >>> lower, right? Thanks, >> >> Nope, that generation 19 is valid. That fs has a higher generation, so >> that's completely valid. >> >> The generation 19 is there because there is another csum leaf whose >> generation is 19. >> > > Then this patch does nothing, because we already have this check lower, > so how exactly did it make the panic go away? Thanks, > > Josef Sorry, I don't get your point. The generation 19 isn't larger than last_trans_committed, so that check has nothing to do with this case. And then it goes to the header_nritems() check, which is 0, and with first_key present, which is invalid and we error out, rejecting the corrupted leaf. What's the problem then? Thanks, Qu
On 8/11/20 8:29 PM, Qu Wenruo wrote: > > > On 2020/8/12 上午8:23, Josef Bacik wrote: >> On 8/11/20 7:04 PM, Qu Wenruo wrote: >>> >>> > [...] >>>> Which I assume is the problem? The generation is 19, is that > >>>> last_trans_committed? Seems like this check just needs to be moved >>>> lower, right? Thanks, >>> >>> Nope, that generation 19 is valid. That fs has a higher generation, so >>> that's completely valid. >>> >>> The generation 19 is there because there is another csum leaf whose >>> generation is 19. >>> >> >> Then this patch does nothing, because we already have this check lower, >> so how exactly did it make the panic go away? Thanks, >> >> Josef > > Sorry, I don't get your point. > > The generation 19 isn't larger than last_trans_committed, so that check > has nothing to do with this case. > > And then it goes to the header_nritems() check, which is 0, and with > first_key present, which is invalid and we error out, rejecting the > corrupted leaf. > > What's the problem then? /* We have @first_key, so this @eb must have at least one item */ if (btrfs_header_nritems(eb) == 0) { btrfs_err(fs_info, "invalid tree nritems, bytenr=%llu nritems=0 expect >0", eb->start); WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)); return -EUCLEAN; } /* * For live tree block (new tree blocks in current transaction), * we need proper lock context to avoid race, which is impossible here. * So we only checks tree blocks which is read from disk, whose * generation <= fs_info->last_trans_committed. */ if (btrfs_header_generation(eb) > fs_info->last_trans_committed) return 0; /* We have @first_key, so this @eb must have at least one item */ if (btrfs_header_nritems(eb) == 0) { btrfs_err(fs_info, "invalid tree nritems, bytenr=%llu nritems=0 expect >0", eb->start); WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)); return -EUCLEAN; } This is the code, you have the exact same check above the header generation thing, and that's not the problem, so I don't understand why you have added this check. Josef
On 2020/8/12 上午9:50, Josef Bacik wrote: > On 8/11/20 8:29 PM, Qu Wenruo wrote: >> >> >> On 2020/8/12 上午8:23, Josef Bacik wrote: >>> On 8/11/20 7:04 PM, Qu Wenruo wrote: >>>> >>>> >> [...] >>>>> Which I assume is the problem? The generation is 19, is that > >>>>> last_trans_committed? Seems like this check just needs to be moved >>>>> lower, right? Thanks, >>>> >>>> Nope, that generation 19 is valid. That fs has a higher generation, so >>>> that's completely valid. >>>> >>>> The generation 19 is there because there is another csum leaf whose >>>> generation is 19. >>>> >>> >>> Then this patch does nothing, because we already have this check lower, >>> so how exactly did it make the panic go away? Thanks, >>> >>> Josef >> >> Sorry, I don't get your point. >> >> The generation 19 isn't larger than last_trans_committed, so that check >> has nothing to do with this case. >> >> And then it goes to the header_nritems() check, which is 0, and with >> first_key present, which is invalid and we error out, rejecting the >> corrupted leaf. >> >> What's the problem then? > > > /* We have @first_key, so this @eb must have at least one item */ > if (btrfs_header_nritems(eb) == 0) { > btrfs_err(fs_info, > "invalid tree nritems, bytenr=%llu nritems=0 expect >0", > eb->start); > WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)); > return -EUCLEAN; > } > > /* > * For live tree block (new tree blocks in current transaction), > * we need proper lock context to avoid race, which is > impossible here. > * So we only checks tree blocks which is read from disk, whose > * generation <= fs_info->last_trans_committed. > */ > if (btrfs_header_generation(eb) > fs_info->last_trans_committed) > return 0; > > /* We have @first_key, so this @eb must have at least one item */ > if (btrfs_header_nritems(eb) == 0) { > btrfs_err(fs_info, > "invalid tree nritems, bytenr=%llu nritems=0 expect >0", > eb->start); > WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)); > return -EUCLEAN; > } > > > This is the code, you have the exact same check above the header > generation thing, and that's not the problem, so I don't understand why > you have added this check. Oh, you're right. A bad auto merge, which we have merged the same patch before... Thanks for spotting this! Qu > > Josef >
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index c850d7f44fbe..b12804d1faac 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -393,6 +393,14 @@ int btrfs_verify_level_key(struct extent_buffer *eb, int level, if (!first_key) return 0; + /* We have @first_key, so this @eb must have at least one item */ + if (btrfs_header_nritems(eb) == 0) { + btrfs_err(fs_info, + "invalid tree nritems, bytenr=%llu nritems=0 expect >0", + eb->start); + WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)); + return -EUCLEAN; + } /* * For live tree block (new tree blocks in current transaction),
[BUG] With crafted image, btrfs will panic at btree operations: kernel BUG at fs/btrfs/ctree.c:3894! invalid opcode: 0000 [#1] SMP PTI CPU: 0 PID: 1138 Comm: btrfs-transacti Not tainted 5.0.0-rc8+ #9 RIP: 0010:__push_leaf_left+0x6b6/0x6e0 Code: 00 00 48 98 48 8d 04 80 48 8d 74 80 65 e8 42 5a 04 00 48 8b bd 78 ff ff ff 8b bf 90 d0 00 00 89 7d 98 83 ef 65 e9 06 ff ff ff <0f> 0b 0f 0b 48 8b 85 78 ff ff ff 8b 90 90 d0 00 00 e9 eb fe ff ff RSP: 0018:ffffc0bd4128b990 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffffa0a4ab8f0e38 RCX: 0000000000000000 RDX: ffffa0a280000000 RSI: 0000000000000000 RDI: ffffa0a4b3814000 RBP: ffffc0bd4128ba38 R08: 0000000000001000 R09: ffffc0bd4128b948 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000240 R13: ffffa0a4b556fb60 R14: ffffa0a4ab8f0af0 R15: ffffa0a4ab8f0af0 FS: 0000000000000000(0000) GS:ffffa0a4b7a00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f2461c80020 CR3: 000000022b32a006 CR4: 00000000000206f0 Call Trace: ? _cond_resched+0x1a/0x50 push_leaf_left+0x179/0x190 btrfs_del_items+0x316/0x470 btrfs_del_csums+0x215/0x3a0 __btrfs_free_extent.isra.72+0x5a7/0xbe0 __btrfs_run_delayed_refs+0x539/0x1120 btrfs_run_delayed_refs+0xdb/0x1b0 btrfs_commit_transaction+0x52/0x950 ? start_transaction+0x94/0x450 transaction_kthread+0x163/0x190 kthread+0x105/0x140 ? btrfs_cleanup_transaction+0x560/0x560 ? kthread_destroy_worker+0x50/0x50 ret_from_fork+0x35/0x40 Modules linked in: ---[ end trace c2425e6e89b5558f ]--- [CAUSE] The offending csum tree looks like this: checksum tree key (CSUM_TREE ROOT_ITEM 0) node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE ... key (EXTENT_CSUM EXTENT_CSUM 85975040) block 29630464 gen 17 key (EXTENT_CSUM EXTENT_CSUM 89911296) block 29642752 gen 17 <<< key (EXTENT_CSUM EXTENT_CSUM 92274688) block 29646848 gen 17 ... leaf 29630464 items 6 free space 1 generation 17 owner CSUM_TREE item 0 key (EXTENT_CSUM EXTENT_CSUM 85975040) itemoff 3987 itemsize 8 range start 85975040 end 85983232 length 8192 ... leaf 29642752 items 0 free space 3995 generation 17 owner 0 ^ empty leaf invalid owner ^ leaf 29646848 items 1 free space 602 generation 17 owner CSUM_TREE item 0 key (EXTENT_CSUM EXTENT_CSUM 92274688) itemoff 627 itemsize 3368 range start 92274688 end 95723520 length 3448832 So we have a corrupted csum tree where one tree leaf is completely empty, causing unbalanced btree, thus leading to unexpected btree balance error. [FIX] For this particular case, we handle it in two directions to catch it: - Check if the tree block is empty through btrfs_verify_level_key() So that invalid tree blocks won't be read out through btrfs_search_slot() and its variants. - Check 0 tree owner in tree checker NO tree is using 0 as its tree owner, detect it and reject at tree block read time. Link: https://bugzilla.kernel.org/show_bug.cgi?id=202821 Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/disk-io.c | 8 ++++++++ 1 file changed, 8 insertions(+)