| Message ID | 20220311104558.157283-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ceph: dencrypt the dentry names early and once for readdir | expand |
Hi Xiubo,
I've started reviewing this patchset but, while running some quick tests,
I've seen found a bug. I was testing with fstests, but I can easily
reproduce it simply using 'src/dirhash_collide', from fstests:
- mount filesystem and create an encrypted directory
- run that application on the encrypted (unlocked) directory:
# cd mydir
# $FSTESTSDIR/src/dirhash_collide -d -n 10000 .
- umount filesystem and mount it again
- do an 'ls' in 'mydir' and I get:
[ 115.807181] ------------[ cut here ]------------
[ 115.807865] WARNING: CPU: 0 PID: 220 at fs/ceph/crypto.c:138 ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
[ 115.809298] Modules linked in: ceph libceph
[ 115.809881] CPU: 0 PID: 220 Comm: ls Not tainted 5.17.0-rc6+ #91
[ 115.810720] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
[ 115.812298] RIP: 0010:ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
[ 115.813238] Code: 48 8b 44 24 50 48 89 85 ad 00 00 00 48 8b 44 24 58 48 89 85 b5 00 00 00 e9 2f ff ff ff 48 89 ef e8 6f dd 17 e1 e9 45 ff ff ff <0f> 0b e9 a2 fe ff ff 417
[ 115.815800] RSP: 0018:ffffc90000487bc0 EFLAGS: 00010246
[ 115.816519] RAX: 0000000000000000 RBX: 1ffff92000090f78 RCX: ffffffffa014654e
[ 115.817502] RDX: dffffc0000000000 RSI: ffffc90000487d58 RDI: ffff8880791b3230
[ 115.818466] RBP: ffffc90000487e90 R08: 0000000000000007 R09: ffff8880605d8748
[ 115.819445] R10: fffffbfff05a79b3 R11: 0000000000000001 R12: ffff8880791b2fe8
[ 115.820437] R13: ffff88807f88ae00 R14: ffffc90000487d58 R15: 0000000000000000
[ 115.821424] FS: 00007fe454541800(0000) GS:ffff888060c00000(0000) knlGS:0000000000000000
[ 115.822526] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 115.823308] CR2: 00007fe454530058 CR3: 000000000d6a0000 CR4: 00000000000006b0
[ 115.824296] Call Trace:
[ 115.824646] <TASK>
[ 115.824963] ? ceph_fscrypt_as_ctx_to_req+0x40/0x40 [ceph]
[ 115.825774] ? create_object.isra.0+0x34a/0x4b0
[ 115.826418] ? preempt_count_sub+0x18/0xc0
[ 115.827069] ? _raw_spin_unlock_irqrestore+0x28/0x40
[ 115.827815] ceph_readdir+0xe24/0x1fa0 [ceph]
[ 115.828459] ? preempt_count_sub+0x18/0xc0
[ 115.829028] ? preempt_count_sub+0x18/0xc0
[ 115.829604] ? ceph_d_revalidate+0x970/0x970 [ceph]
[ 115.830314] ? down_write_killable+0xb2/0x110
[ 115.830921] ? __down_killable+0x200/0x200
[ 115.831488] iterate_dir+0xe9/0x2a0
[ 115.831995] __x64_sys_getdents64+0xf2/0x1c0
[ 115.832587] ? __x64_sys_getdents+0x1c0/0x1c0
[ 115.833203] ? handle_mm_fault+0x1c3/0x230
[ 115.833778] ? compat_fillonedir+0x1b0/0x1b0
[ 115.834419] ? do_user_addr_fault+0x32b/0x940
[ 115.835048] do_syscall_64+0x43/0x90
[ 115.835575] entry_SYSCALL_64_after_hwframe+0x44/0xae
After this, I also see some KASAN NULL pointers, but I assume it's the
result of the above bug.
Cheers,
On 3/12/22 12:02 AM, Luís Henriques wrote: > Hi Xiubo, > > I've started reviewing this patchset but, while running some quick tests, > I've seen found a bug. I was testing with fstests, but I can easily > reproduce it simply using 'src/dirhash_collide', from fstests: > > - mount filesystem and create an encrypted directory > > - run that application on the encrypted (unlocked) directory: > # cd mydir > # $FSTESTSDIR/src/dirhash_collide -d -n 10000 . > > - umount filesystem and mount it again > > - do an 'ls' in 'mydir' and I get: > > [ 115.807181] ------------[ cut here ]------------ > [ 115.807865] WARNING: CPU: 0 PID: 220 at fs/ceph/crypto.c:138 ceph_encode_encrypted_dname+0x1e6/0x240 [ceph] > [ 115.809298] Modules linked in: ceph libceph > [ 115.809881] CPU: 0 PID: 220 Comm: ls Not tainted 5.17.0-rc6+ #91 > [ 115.810720] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014 > [ 115.812298] RIP: 0010:ceph_encode_encrypted_dname+0x1e6/0x240 [ceph] > [ 115.813238] Code: 48 8b 44 24 50 48 89 85 ad 00 00 00 48 8b 44 24 58 48 89 85 b5 00 00 00 e9 2f ff ff ff 48 89 ef e8 6f dd 17 e1 e9 45 ff ff ff <0f> 0b e9 a2 fe ff ff 417 > [ 115.815800] RSP: 0018:ffffc90000487bc0 EFLAGS: 00010246 > [ 115.816519] RAX: 0000000000000000 RBX: 1ffff92000090f78 RCX: ffffffffa014654e > [ 115.817502] RDX: dffffc0000000000 RSI: ffffc90000487d58 RDI: ffff8880791b3230 > [ 115.818466] RBP: ffffc90000487e90 R08: 0000000000000007 R09: ffff8880605d8748 > [ 115.819445] R10: fffffbfff05a79b3 R11: 0000000000000001 R12: ffff8880791b2fe8 > [ 115.820437] R13: ffff88807f88ae00 R14: ffffc90000487d58 R15: 0000000000000000 > [ 115.821424] FS: 00007fe454541800(0000) GS:ffff888060c00000(0000) knlGS:0000000000000000 > [ 115.822526] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 115.823308] CR2: 00007fe454530058 CR3: 000000000d6a0000 CR4: 00000000000006b0 > [ 115.824296] Call Trace: > [ 115.824646] <TASK> > [ 115.824963] ? ceph_fscrypt_as_ctx_to_req+0x40/0x40 [ceph] > [ 115.825774] ? create_object.isra.0+0x34a/0x4b0 > [ 115.826418] ? preempt_count_sub+0x18/0xc0 > [ 115.827069] ? _raw_spin_unlock_irqrestore+0x28/0x40 > [ 115.827815] ceph_readdir+0xe24/0x1fa0 [ceph] > [ 115.828459] ? preempt_count_sub+0x18/0xc0 > [ 115.829028] ? preempt_count_sub+0x18/0xc0 > [ 115.829604] ? ceph_d_revalidate+0x970/0x970 [ceph] > [ 115.830314] ? down_write_killable+0xb2/0x110 > [ 115.830921] ? __down_killable+0x200/0x200 > [ 115.831488] iterate_dir+0xe9/0x2a0 > [ 115.831995] __x64_sys_getdents64+0xf2/0x1c0 > [ 115.832587] ? __x64_sys_getdents+0x1c0/0x1c0 > [ 115.833203] ? handle_mm_fault+0x1c3/0x230 > [ 115.833778] ? compat_fillonedir+0x1b0/0x1b0 > [ 115.834419] ? do_user_addr_fault+0x32b/0x940 > [ 115.835048] do_syscall_64+0x43/0x90 > [ 115.835575] entry_SYSCALL_64_after_hwframe+0x44/0xae > > After this, I also see some KASAN NULL pointers, but I assume it's the > result of the above bug. Hi Luis, Thanks for your feedback. Yesterday I have test this by using both the fsstress and xfstests for 1 hour, didn't see this call trace. I will check it. - XIubo > Cheers,
From: Xiubo Li <xiubli@redhat.com> This is a new approach to improve the readdir and based the previous discussion in another thread: https://patchwork.kernel.org/project/ceph-devel/list/?series=621901 Just start a new thread for this. As Jeff suggested, this patch series will dentrypt the dentry name during parsing the readdir data in handle_reply(). And then in both ceph_readdir_prepopulate() and ceph_readdir() we will use the dencrypted name directly. NOTE: we will base64_dencode and dencrypt the names in-place instead of allocating tmp buffers. For base64_dencode it's safe because the dencoded string buffer will always be shorter. Xiubo Li (4): ceph: pass the request to parse_reply_info_readdir() ceph: add ceph_encode_encrypted_dname() helper ceph: dencrypt the dentry names early and once for readdir ceph: clean up the ceph_readdir() code fs/ceph/crypto.c | 18 +++++--- fs/ceph/crypto.h | 2 + fs/ceph/dir.c | 64 +++++++++------------------ fs/ceph/inode.c | 37 ++-------------- fs/ceph/mds_client.c | 101 ++++++++++++++++++++++++++++++++++++------- fs/ceph/mds_client.h | 4 +- 6 files changed, 127 insertions(+), 99 deletions(-)