Re: [PATCH v3] f2fs: allocate buffer for decrypting filename to avoid panic
On 2018/2/28 10:19, Yunlong Song wrote:
> In some platforms (such as arm), high memory is used, then the
> decrypting filename will cause panic, the reason see commit
> 569cf1876a32e574ba8a7fb825cd91bafd003882 ("f2fs crypto: allocate buffer
> for decrypting filename"):
>
> We got dentry pages from high_mem, and its address space directly goes into
> the
> decryption path via f2fs_fname_disk_to_usr.
> But, sg_init_one assumes the address is not from high_mem, so we can get this
> panic since it doesn't call kmap_high but kunmap_high is triggered at the
> end.
>
> kernel BUG at ../../../../../../kernel/mm/highmem.c:290!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> ...
> (kunmap_high+0xb0/0xb8) from [] (__kunmap_atomic+0xa0/0xa4)
> (__kunmap_atomic+0xa0/0xa4) from []
> (blkcipher_walk_done+0x128/0x1ec)
> (blkcipher_walk_done+0x128/0x1ec) from []
> (crypto_cbc_decrypt+0xc0/0x170)
> (crypto_cbc_decrypt+0xc0/0x170) from []
> (crypto_cts_decrypt+0xc0/0x114)
> (crypto_cts_decrypt+0xc0/0x114) from [] (async_decrypt+0x40/0x48)
> (async_decrypt+0x40/0x48) from []
> (f2fs_fname_disk_to_usr+0x124/0x304)
> (f2fs_fname_disk_to_usr+0x124/0x304) from []
> (f2fs_fill_dentries+0xac/0x188)
> (f2fs_fill_dentries+0xac/0x188) from [] (f2fs_readdir+0x1f0/0x300)
> (f2fs_readdir+0x1f0/0x300) from [] (vfs_readdir+0x90/0xb4)
> (vfs_readdir+0x90/0xb4) from [] (SyS_getdents64+0x64/0xcc)
> (SyS_getdents64+0x64/0xcc) from [] (ret_fast_syscall+0x0/0x30)
>
> Howerver, later patches:
> commit e06f86e61d7a67fe6e826010f57aa39c674f4b1b ("f2fs crypto: avoid
> unneeded memory allocation in ->readdir")
>
> reverts the codes, which causes panic again in arm, so let's add part of
> the old patch again for dentry page.
>
Fixes: e06f86e61d7a ("f2fs crypto: avoid unneeded memory allocation in
->readdir")
> Signed-off-by: Yunlong Song
Looks good to me, you can add:
Reviewed-by: Chao Yu
Minor, we can just use 'git revert' to generate patch, so that commit title can
notice the patch are reverting buggy commit.
Thanks,
Re: [PATCH v3] f2fs: allocate buffer for decrypting filename to avoid panic
On 2018/2/28 10:19, Yunlong Song wrote:
> In some platforms (such as arm), high memory is used, then the
> decrypting filename will cause panic, the reason see commit
> 569cf1876a32e574ba8a7fb825cd91bafd003882 ("f2fs crypto: allocate buffer
> for decrypting filename"):
>
> We got dentry pages from high_mem, and its address space directly goes into
> the
> decryption path via f2fs_fname_disk_to_usr.
> But, sg_init_one assumes the address is not from high_mem, so we can get this
> panic since it doesn't call kmap_high but kunmap_high is triggered at the
> end.
>
> kernel BUG at ../../../../../../kernel/mm/highmem.c:290!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> ...
> (kunmap_high+0xb0/0xb8) from [] (__kunmap_atomic+0xa0/0xa4)
> (__kunmap_atomic+0xa0/0xa4) from []
> (blkcipher_walk_done+0x128/0x1ec)
> (blkcipher_walk_done+0x128/0x1ec) from []
> (crypto_cbc_decrypt+0xc0/0x170)
> (crypto_cbc_decrypt+0xc0/0x170) from []
> (crypto_cts_decrypt+0xc0/0x114)
> (crypto_cts_decrypt+0xc0/0x114) from [] (async_decrypt+0x40/0x48)
> (async_decrypt+0x40/0x48) from []
> (f2fs_fname_disk_to_usr+0x124/0x304)
> (f2fs_fname_disk_to_usr+0x124/0x304) from []
> (f2fs_fill_dentries+0xac/0x188)
> (f2fs_fill_dentries+0xac/0x188) from [] (f2fs_readdir+0x1f0/0x300)
> (f2fs_readdir+0x1f0/0x300) from [] (vfs_readdir+0x90/0xb4)
> (vfs_readdir+0x90/0xb4) from [] (SyS_getdents64+0x64/0xcc)
> (SyS_getdents64+0x64/0xcc) from [] (ret_fast_syscall+0x0/0x30)
>
> Howerver, later patches:
> commit e06f86e61d7a67fe6e826010f57aa39c674f4b1b ("f2fs crypto: avoid
> unneeded memory allocation in ->readdir")
>
> reverts the codes, which causes panic again in arm, so let's add part of
> the old patch again for dentry page.
>
Fixes: e06f86e61d7a ("f2fs crypto: avoid unneeded memory allocation in
->readdir")
> Signed-off-by: Yunlong Song
Looks good to me, you can add:
Reviewed-by: Chao Yu
Minor, we can just use 'git revert' to generate patch, so that commit title can
notice the patch are reverting buggy commit.
Thanks,
[PATCH v3] f2fs: allocate buffer for decrypting filename to avoid panic
In some platforms (such as arm), high memory is used, then the
decrypting filename will cause panic, the reason see commit
569cf1876a32e574ba8a7fb825cd91bafd003882 ("f2fs crypto: allocate buffer
for decrypting filename"):
We got dentry pages from high_mem, and its address space directly goes into the
decryption path via f2fs_fname_disk_to_usr.
But, sg_init_one assumes the address is not from high_mem, so we can get this
panic since it doesn't call kmap_high but kunmap_high is triggered at the end.
kernel BUG at ../../../../../../kernel/mm/highmem.c:290!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
(kunmap_high+0xb0/0xb8) from [] (__kunmap_atomic+0xa0/0xa4)
(__kunmap_atomic+0xa0/0xa4) from []
(blkcipher_walk_done+0x128/0x1ec)
(blkcipher_walk_done+0x128/0x1ec) from []
(crypto_cbc_decrypt+0xc0/0x170)
(crypto_cbc_decrypt+0xc0/0x170) from []
(crypto_cts_decrypt+0xc0/0x114)
(crypto_cts_decrypt+0xc0/0x114) from [] (async_decrypt+0x40/0x48)
(async_decrypt+0x40/0x48) from []
(f2fs_fname_disk_to_usr+0x124/0x304)
(f2fs_fname_disk_to_usr+0x124/0x304) from []
(f2fs_fill_dentries+0xac/0x188)
(f2fs_fill_dentries+0xac/0x188) from [] (f2fs_readdir+0x1f0/0x300)
(f2fs_readdir+0x1f0/0x300) from [] (vfs_readdir+0x90/0xb4)
(vfs_readdir+0x90/0xb4) from [] (SyS_getdents64+0x64/0xcc)
(SyS_getdents64+0x64/0xcc) from [] (ret_fast_syscall+0x0/0x30)
Howerver, later patches:
commit e06f86e61d7a67fe6e826010f57aa39c674f4b1b ("f2fs crypto: avoid
unneeded memory allocation in ->readdir")
reverts the codes, which causes panic again in arm, so let's add part of
the old patch again for dentry page.
Signed-off-by: Yunlong Song
---
fs/f2fs/dir.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index f00b5ed..de2e295 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -825,9 +825,16 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct
f2fs_dentry_ptr *d,
int save_len = fstr->len;
int err;
+ de_name.name = f2fs_kmalloc(sbi, de_name.len, GFP_NOFS);
+ if (!de_name.name)
+ return -ENOMEM;
+
+ memcpy(de_name.name, d->filename[bit_pos], de_name.len);
+
err = fscrypt_fname_disk_to_usr(d->inode,
(u32)de->hash_code, 0,
_name, fstr);
+ kfree(de_name.name);
if (err)
return err;
--
1.8.5.2
[PATCH v3] f2fs: allocate buffer for decrypting filename to avoid panic
In some platforms (such as arm), high memory is used, then the
decrypting filename will cause panic, the reason see commit
569cf1876a32e574ba8a7fb825cd91bafd003882 ("f2fs crypto: allocate buffer
for decrypting filename"):
We got dentry pages from high_mem, and its address space directly goes into the
decryption path via f2fs_fname_disk_to_usr.
But, sg_init_one assumes the address is not from high_mem, so we can get this
panic since it doesn't call kmap_high but kunmap_high is triggered at the end.
kernel BUG at ../../../../../../kernel/mm/highmem.c:290!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
(kunmap_high+0xb0/0xb8) from [] (__kunmap_atomic+0xa0/0xa4)
(__kunmap_atomic+0xa0/0xa4) from []
(blkcipher_walk_done+0x128/0x1ec)
(blkcipher_walk_done+0x128/0x1ec) from []
(crypto_cbc_decrypt+0xc0/0x170)
(crypto_cbc_decrypt+0xc0/0x170) from []
(crypto_cts_decrypt+0xc0/0x114)
(crypto_cts_decrypt+0xc0/0x114) from [] (async_decrypt+0x40/0x48)
(async_decrypt+0x40/0x48) from []
(f2fs_fname_disk_to_usr+0x124/0x304)
(f2fs_fname_disk_to_usr+0x124/0x304) from []
(f2fs_fill_dentries+0xac/0x188)
(f2fs_fill_dentries+0xac/0x188) from [] (f2fs_readdir+0x1f0/0x300)
(f2fs_readdir+0x1f0/0x300) from [] (vfs_readdir+0x90/0xb4)
(vfs_readdir+0x90/0xb4) from [] (SyS_getdents64+0x64/0xcc)
(SyS_getdents64+0x64/0xcc) from [] (ret_fast_syscall+0x0/0x30)
Howerver, later patches:
commit e06f86e61d7a67fe6e826010f57aa39c674f4b1b ("f2fs crypto: avoid
unneeded memory allocation in ->readdir")
reverts the codes, which causes panic again in arm, so let's add part of
the old patch again for dentry page.
Signed-off-by: Yunlong Song
---
fs/f2fs/dir.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index f00b5ed..de2e295 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -825,9 +825,16 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct
f2fs_dentry_ptr *d,
int save_len = fstr->len;
int err;
+ de_name.name = f2fs_kmalloc(sbi, de_name.len, GFP_NOFS);
+ if (!de_name.name)
+ return -ENOMEM;
+
+ memcpy(de_name.name, d->filename[bit_pos], de_name.len);
+
err = fscrypt_fname_disk_to_usr(d->inode,
(u32)de->hash_code, 0,
_name, fstr);
+ kfree(de_name.name);
if (err)
return err;
--
1.8.5.2
