In bcachefs, we've been getting reports of memory corruption, and I finally came
up with a way to reproduce it - fstests generic/269, with encryption enabled
(bcachefs uses chacha20 & poly1305).
It looks like a stray pointer write - I see corruption in a lot of areas, and
it's really easy to spot if I've got kasan, or slub debugging, or pagealloc
debug on - but I haven't been having any luck yet with finding the stray
_write_. kasan does spot stray reads, example below:
It also looks like a regression, but not a recent one - I tested as far back as
about 5.10 before I ran into problems with the kernel building with my current
toolchain, feh.
Example kasan splat below - as you can see we're in skcipher_next_slow(). I
think what's going on, and the reason bcachefs is hitting this and not other
chacha20 users, is that the lengths we're encrypting aren't aligned to the
chacha20 blocksize. We're hitting this in the bcachefs btree io path, not the
normal user data path where the crypto calls are chacha20 blocksize aligned.
Any chance one of you could help with debugging this? I'd be happy to get you
whatever info you need, or help you reproduce it with ktest [1]
1: https://evilpiepirate.org/git/ktest.git/
00024 BUG: KASAN: slab-out-of-bounds in scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
00024 Read of size 32 at addr ffff88806f01afe0 by task fsstress/1436
00024
00024 CPU: 1 PID: 1436 Comm: fsstress Not tainted 5.18.0-01355-gfad6d13aa55f #1
00024 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
00024 Call Trace:
00024 <TASK>
00024 dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
00024 print_report.cold (mm/kasan/report.c:314 mm/kasan/report.c:429)
00024 ? scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
00024 kasan_report (mm/kasan/report.c:493)
00024 ? scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
00024 kasan_check_range (mm/kasan/generic.c:190)
00024 memcpy (mm/kasan/shadow.c:65)
00024 scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
00024 skcipher_walk_next (crypto/skcipher.c:280 crypto/skcipher.c:363)
00024 skcipher_walk_done (crypto/skcipher.c:159)
00024 chacha_simd_stream_xor (arch/x86/crypto/chacha_glue.c:174)
00024 ? chacha_crypt_arch (arch/x86/crypto/chacha_glue.c:165)
00024 ? bch2_inode_to_text (fs/bcachefs/inode.c:414)
00024 ? key_type_inline_data_to_text (fs/bcachefs/bkey_methods.c:127)
00024 ? vfs_truncate (fs/open.c:111)
00024 chacha_simd (arch/x86/crypto/chacha_glue.c:204)
00024 crypto_skcipher_encrypt (crypto/skcipher.c:633)
00024 do_encrypt_sg (fs/bcachefs/checksum.c:107)
00024 ? bch2_chardev_exit (fs/bcachefs/checksum.c:99)
00024 ? validate_bset_keys.constprop.0 (fs/bcachefs/btree_io.c:780)
00024 ? validate_bset (fs/bcachefs/btree_io.c:645)
00024 ? btree_node_sort (fs/bcachefs/btree_io.c:645)
00024 ? bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:260)
00024 ? __bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:259)
00024 ? vmalloc_to_page (mm/vmalloc.c:696)
00024 bch2_encrypt (fs/bcachefs/checksum.c:217)
00024 ? bch2_checksum (fs/bcachefs/checksum.c:213)
00024 __bch2_btree_node_write (fs/bcachefs/btree_io.c:1938)
00024 ? bset_aux_tree_verify (fs/bcachefs/bset.c:385)
00024 ? bch2_btree_complete_write (fs/bcachefs/btree_io.c:1777)
00024 ? bset_aux_tree_buf_end (fs/bcachefs/bset.c:328 (discriminator 2))
00024 ? __kasan_check_write (mm/kasan/shadow.c:38)
00024 ? mutex_unlock (arch/x86/include/asm/atomic64_64.h:190 include/linux/atomic/atomic-long.h:449 include/linux/atomic/atomic-instrumented.h:1790 kernel/locking/mutex.c:178 kernel/locking/mutex.c:537)
00024 ? __mutex_unlock_slowpath.constprop.0 (kernel/locking/mutex.c:535)
00024 ? btree_update_add_key.constprop.0 (fs/bcachefs/keylist.h:25 fs/bcachefs/btree_update_interior.c:519)
00024 bch2_btree_node_write (include/linux/instrumented.h:71 include/asm-generic/bitops/instrumented-non-atomic.h:134 fs/bcachefs/btree_types.h:466 fs/bcachefs/btree_io.c:2096)
00024 btree_split.isra.0 (fs/bcachefs/btree_update_interior.c:1460)
00024 ? bch2_btree_update_start (fs/bcachefs/btree_update_interior.c:1079)
00024 ? __btree_split_node (fs/bcachefs/btree_update_interior.c:973)
00024 ? bch2_btree_node_alloc_replacement (fs/bcachefs/btree_update_interior.c:1397)
00024 bch2_btree_split_leaf (fs/bcachefs/btree_update_interior.c:1606)
00024 ? bch2_inode_v2_invalid (fs/bcachefs/inode.c:363)
00024 bch2_trans_commit_error (fs/bcachefs/btree_update_leaf.c:970)
00024 ? bch2_setattr_nonsize (fs/bcachefs/btree_update.h:106 fs/bcachefs/fs.c:752)
00024 ? bch2_trans_update_by_path (fs/bcachefs/btree_update_leaf.c:964)
00024 ? six_unlock_write (arch/x86/include/asm/atomic64_64.h:160 include/linux/atomic/atomic-instrumented.h:699 kernel/locking/six.c:572 kernel/locking/six.c:607)
00024 ? bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:260)
00024 ? bch2_btree_path_verify_locks (fs/bcachefs/btree_iter.c:401 fs/bcachefs/btree_iter.c:391)
00024 __bch2_trans_commit (fs/bcachefs/btree_update_leaf.c:1182)
00024 ? bch2_btree_add_journal_pin (fs/bcachefs/btree_update_leaf.c:1084)
00024 bch2_setattr_nonsize (fs/bcachefs/btree_update.h:106 fs/bcachefs/fs.c:752)
00024 ? xas_start (lib/xarray.c:193)
00024 ? bch2_unlink (fs/bcachefs/fs.c:707)
00024 ? bch2_inode_peek (fs/bcachefs/inode.c:262)
00024 ? truncate_inode_partial_folio (mm/truncate.c:332)
00024 ? __kernel_text_address (kernel/extable.c:79)
00024 ? truncate_pagecache (mm/truncate.c:755)
00024 bch2_truncate (fs/bcachefs/fs-io.c:2721)
00024 ? bch2_fsync (fs/bcachefs/fs-io.c:2622)
00024 ? __kasan_check_write (mm/kasan/shadow.c:38)
00024 ? lockref_get_not_dead (lib/lockref.c:203 (discriminator 7))
00024 ? setattr_prepare (fs/attr.c:108)
00024 ? find_inode_rcu (fs/inode.c:2455)
00024 bch2_setattr (fs/bcachefs/fs.c:830)
00024 notify_change (fs/attr.c:414)
00024 do_truncate (fs/open.c:66)
00024 ? file_open_root (fs/open.c:41)
00024 ? get_acl (fs/posix_acl.c:121)
00024 ? inode_permission (fs/namei.c:533)
00024 vfs_truncate (fs/open.c:111)
00024 do_sys_truncate.part.0 (fs/open.c:135)
00024 ? vfs_truncate (fs/open.c:122)
00024 ? __kasan_check_read (mm/kasan/shadow.c:32)
00024 ? fpregs_assert_state_consistent (arch/x86/kernel/fpu/context.h:39 arch/x86/kernel/fpu/core.c:757)
00024 __x64_sys_truncate (fs/open.c:144)
00024 do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
00024 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
On Thu, Jun 02, 2022 at 07:25:48PM -0700, Eric Biggers wrote:
> What if the address is in the vmalloc area and spans a page boundary? If the
> next page isn't physically contiguous, this will write to some random page. A
> multi-element scatterlist would be needed to describe all the physical pages.
That's exactly it :) Can't believe I missed that, or that that bug was lurking
for so long.
> Note that in v5.5 and later, ChaCha20 support is available via the library
> function chacha20_crypt(), in addition to the skcipher API. You should consider
> using the library function, as it seems to match your use case.
I'll check that out - that looks like exactly what I was looking for when I
originally wrote this code.
Thanks, I owe you a beer :)
On Thu, Jun 02, 2022 at 10:11:34PM -0400, Kent Overstreet wrote:
> In bcachefs, we've been getting reports of memory corruption, and I finally came
> up with a way to reproduce it - fstests generic/269, with encryption enabled
> (bcachefs uses chacha20 & poly1305).
>
> It looks like a stray pointer write - I see corruption in a lot of areas, and
> it's really easy to spot if I've got kasan, or slub debugging, or pagealloc
> debug on - but I haven't been having any luck yet with finding the stray
> _write_. kasan does spot stray reads, example below:
>
> It also looks like a regression, but not a recent one - I tested as far back as
> about 5.10 before I ran into problems with the kernel building with my current
> toolchain, feh.
>
> Example kasan splat below - as you can see we're in skcipher_next_slow(). I
> think what's going on, and the reason bcachefs is hitting this and not other
> chacha20 users, is that the lengths we're encrypting aren't aligned to the
> chacha20 blocksize. We're hitting this in the bcachefs btree io path, not the
> normal user data path where the crypto calls are chacha20 blocksize aligned.
>
> Any chance one of you could help with debugging this? I'd be happy to get you
> whatever info you need, or help you reproduce it with ktest [1]
>
> 1: https://evilpiepirate.org/git/ktest.git/
>
> 00024 BUG: KASAN: slab-out-of-bounds in scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
> 00024 Read of size 32 at addr ffff88806f01afe0 by task fsstress/1436
> 00024
> 00024 CPU: 1 PID: 1436 Comm: fsstress Not tainted 5.18.0-01355-gfad6d13aa55f #1
> 00024 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
> 00024 Call Trace:
> 00024 <TASK>
> 00024 dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
> 00024 print_report.cold (mm/kasan/report.c:314 mm/kasan/report.c:429)
> 00024 ? scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
> 00024 kasan_report (mm/kasan/report.c:493)
> 00024 ? scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
> 00024 kasan_check_range (mm/kasan/generic.c:190)
> 00024 memcpy (mm/kasan/shadow.c:65)
> 00024 scatterwalk_copychunks (include/linux/uaccess.h:184 include/linux/uaccess.h:211 include/linux/highmem-internal.h:227 include/crypto/scatterwalk.h:62 crypto/scatterwalk.c:39)
> 00024 skcipher_walk_next (crypto/skcipher.c:280 crypto/skcipher.c:363)
> 00024 skcipher_walk_done (crypto/skcipher.c:159)
> 00024 chacha_simd_stream_xor (arch/x86/crypto/chacha_glue.c:174)
> 00024 ? chacha_crypt_arch (arch/x86/crypto/chacha_glue.c:165)
> 00024 ? bch2_inode_to_text (fs/bcachefs/inode.c:414)
> 00024 ? key_type_inline_data_to_text (fs/bcachefs/bkey_methods.c:127)
> 00024 ? vfs_truncate (fs/open.c:111)
> 00024 chacha_simd (arch/x86/crypto/chacha_glue.c:204)
> 00024 crypto_skcipher_encrypt (crypto/skcipher.c:633)
> 00024 do_encrypt_sg (fs/bcachefs/checksum.c:107)
> 00024 ? bch2_chardev_exit (fs/bcachefs/checksum.c:99)
> 00024 ? validate_bset_keys.constprop.0 (fs/bcachefs/btree_io.c:780)
> 00024 ? validate_bset (fs/bcachefs/btree_io.c:645)
> 00024 ? btree_node_sort (fs/bcachefs/btree_io.c:645)
> 00024 ? bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:260)
> 00024 ? __bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:259)
> 00024 ? vmalloc_to_page (mm/vmalloc.c:696)
> 00024 bch2_encrypt (fs/bcachefs/checksum.c:217)
> 00024 ? bch2_checksum (fs/bcachefs/checksum.c:213)
> 00024 __bch2_btree_node_write (fs/bcachefs/btree_io.c:1938)
> 00024 ? bset_aux_tree_verify (fs/bcachefs/bset.c:385)
> 00024 ? bch2_btree_complete_write (fs/bcachefs/btree_io.c:1777)
> 00024 ? bset_aux_tree_buf_end (fs/bcachefs/bset.c:328 (discriminator 2))
> 00024 ? __kasan_check_write (mm/kasan/shadow.c:38)
> 00024 ? mutex_unlock (arch/x86/include/asm/atomic64_64.h:190 include/linux/atomic/atomic-long.h:449 include/linux/atomic/atomic-instrumented.h:1790 kernel/locking/mutex.c:178 kernel/locking/mutex.c:537)
> 00024 ? __mutex_unlock_slowpath.constprop.0 (kernel/locking/mutex.c:535)
> 00024 ? btree_update_add_key.constprop.0 (fs/bcachefs/keylist.h:25 fs/bcachefs/btree_update_interior.c:519)
> 00024 bch2_btree_node_write (include/linux/instrumented.h:71 include/asm-generic/bitops/instrumented-non-atomic.h:134 fs/bcachefs/btree_types.h:466 fs/bcachefs/btree_io.c:2096)
> 00024 btree_split.isra.0 (fs/bcachefs/btree_update_interior.c:1460)
> 00024 ? bch2_btree_update_start (fs/bcachefs/btree_update_interior.c:1079)
> 00024 ? __btree_split_node (fs/bcachefs/btree_update_interior.c:973)
> 00024 ? bch2_btree_node_alloc_replacement (fs/bcachefs/btree_update_interior.c:1397)
> 00024 bch2_btree_split_leaf (fs/bcachefs/btree_update_interior.c:1606)
> 00024 ? bch2_inode_v2_invalid (fs/bcachefs/inode.c:363)
> 00024 bch2_trans_commit_error (fs/bcachefs/btree_update_leaf.c:970)
> 00024 ? bch2_setattr_nonsize (fs/bcachefs/btree_update.h:106 fs/bcachefs/fs.c:752)
> 00024 ? bch2_trans_update_by_path (fs/bcachefs/btree_update_leaf.c:964)
> 00024 ? six_unlock_write (arch/x86/include/asm/atomic64_64.h:160 include/linux/atomic/atomic-instrumented.h:699 kernel/locking/six.c:572 kernel/locking/six.c:607)
> 00024 ? bch2_bkey_invalid (fs/bcachefs/bkey_methods.c:260)
> 00024 ? bch2_btree_path_verify_locks (fs/bcachefs/btree_iter.c:401 fs/bcachefs/btree_iter.c:391)
> 00024 __bch2_trans_commit (fs/bcachefs/btree_update_leaf.c:1182)
> 00024 ? bch2_btree_add_journal_pin (fs/bcachefs/btree_update_leaf.c:1084)
> 00024 bch2_setattr_nonsize (fs/bcachefs/btree_update.h:106 fs/bcachefs/fs.c:752)
> 00024 ? xas_start (lib/xarray.c:193)
> 00024 ? bch2_unlink (fs/bcachefs/fs.c:707)
> 00024 ? bch2_inode_peek (fs/bcachefs/inode.c:262)
> 00024 ? truncate_inode_partial_folio (mm/truncate.c:332)
> 00024 ? __kernel_text_address (kernel/extable.c:79)
> 00024 ? truncate_pagecache (mm/truncate.c:755)
> 00024 bch2_truncate (fs/bcachefs/fs-io.c:2721)
> 00024 ? bch2_fsync (fs/bcachefs/fs-io.c:2622)
> 00024 ? __kasan_check_write (mm/kasan/shadow.c:38)
> 00024 ? lockref_get_not_dead (lib/lockref.c:203 (discriminator 7))
> 00024 ? setattr_prepare (fs/attr.c:108)
> 00024 ? find_inode_rcu (fs/inode.c:2455)
> 00024 bch2_setattr (fs/bcachefs/fs.c:830)
> 00024 notify_change (fs/attr.c:414)
> 00024 do_truncate (fs/open.c:66)
> 00024 ? file_open_root (fs/open.c:41)
> 00024 ? get_acl (fs/posix_acl.c:121)
> 00024 ? inode_permission (fs/namei.c:533)
> 00024 vfs_truncate (fs/open.c:111)
> 00024 do_sys_truncate.part.0 (fs/open.c:135)
> 00024 ? vfs_truncate (fs/open.c:122)
> 00024 ? __kasan_check_read (mm/kasan/shadow.c:32)
> 00024 ? fpregs_assert_state_consistent (arch/x86/kernel/fpu/context.h:39 arch/x86/kernel/fpu/core.c:757)
> 00024 __x64_sys_truncate (fs/open.c:144)
> 00024 do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> 00024 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
I assume that the "master" branch of https://github.com/koverstreet/bcachefs
contains the kernel source code that you're testing? The following function
looks suspicious:
static inline int do_encrypt(struct crypto_sync_skcipher *tfm,
struct nonce nonce,
void *buf, size_t len)
{
struct scatterlist sg;
sg_init_table(&sg, 1);
sg_set_page(&sg,
is_vmalloc_addr(buf)
? vmalloc_to_page(buf)
: virt_to_page(buf),
len, offset_in_page(buf));
return do_encrypt_sg(tfm, nonce, &sg, len);
}
What if the address is in the vmalloc area and spans a page boundary? If the
next page isn't physically contiguous, this will write to some random page. A
multi-element scatterlist would be needed to describe all the physical pages.
Note that in v5.5 and later, ChaCha20 support is available via the library
function chacha20_crypt(), in addition to the skcipher API. You should consider
using the library function, as it seems to match your use case.
- Eric