Received: by 10.223.185.116 with SMTP id b49csp3296947wrg; Sun, 25 Feb 2018 19:16:40 -0800 (PST) X-Google-Smtp-Source: AH8x225kOYoinXxahilS9Q20yczUhbpdH89k+pOXOXKIWYMmMhacidJpNba6cZh7hrlcAxOPGKb3 X-Received: by 10.99.109.77 with SMTP id i74mr7098200pgc.73.1519615000863; Sun, 25 Feb 2018 19:16:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519615000; cv=none; d=google.com; s=arc-20160816; b=AH6/hMN5fByRHTN+L+W9qGk3caY59kJ2OkgP69psi0bm4yNZoaYQa2m6UseDLUZlgF M/kCPTGiZ5eOYQwu/kqyBRAaSKn28pmwERClDZEO1YWstyaKEf1Hi8VKGX4qGE1TiJhQ hnTLbqTwit60nQnU8wnlvXYluM3Ub0/BLCK5ARm6JJTAyyt3bk2ub2qQ2sHYi1fwaDVi qbhtmkkbOKGXVzOE/sWcdXiyTa2Ys2pqMSqYVhKOLV98dm0eF21vZHY7/YHF29l0skzV CPhEP5OwmhB/Q47Yig+dDWAPA4B9g1xDdOE5Qi5AVfAtRRxJBN61twtSZLtDEXRkQGxr ZQMw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=g2KKLLdumNjhMd4dioCOetbead/afZdU576OmzYVseo=; b=dJRPRHanbGeOASTeUI0XWjI9x3dRaZZW8FuDgFCc/OuvaiKjcmLFlvZQ/IXTA1vAVj 8tYBpm3SN29KtUOrl4qoIk46qTmLE58QH+nCyvJSLZJ/iIJ3i8kFWklZhPePCs6/SgF6 enPew+GgLrj0Sqgl1q3/i4VCyBpKPV55gS1U1qc512T4hp6nYyMaPRgOeNAj5ANJzFKi HYEdvje2iWP4qwL1Bzf7hfDwMNUKdIP0K+iHQMC1Z1l5IUDFxRcergwnQbbs5aUvAKjV 793x/5+AniXyAIeZlqn3dx29cnvYEVoI9fUh5huTFOiY8a6eaqsjbQiK2gtCOcZDZu7Z s1BA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 15-v6si6060736pla.23.2018.02.25.19.16.26; Sun, 25 Feb 2018 19:16:40 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752057AbeBZDGw (ORCPT + 99 others); Sun, 25 Feb 2018 22:06:52 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:5266 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751945AbeBZDGv (ORCPT ); Sun, 25 Feb 2018 22:06:51 -0500 Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 7ADB5EA9B8681; Mon, 26 Feb 2018 11:06:37 +0800 (CST) Received: from [127.0.0.1] (10.111.220.140) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.361.1; Mon, 26 Feb 2018 11:06:29 +0800 Subject: Re: [PATCH] f2fs: allocate buffer for decrypting filename to avoid panic To: Eric Biggers CC: , , , , , , , , , References: <1519463698-60555-1-git-send-email-yunlong.song@huawei.com> <20180224183230.GA933@zzz.localdomain> From: Yunlong Song Message-ID: <1815fa55-7a32-799a-5f12-45e3d239c469@huawei.com> Date: Mon, 26 Feb 2018 11:06:02 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20180224183230.GA933@zzz.localdomain> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.220.140] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Eric, Thanks for your review, I have removed the symlink part and use kmemdup instead. As for directory pages restricted to lowmem, I am not sure whether it is proper for f2fs, since the directory structures of f2fs are different from ext4. So I just keep its old kmap. On 2018/2/25 2:32, Eric Biggers wrote: > Hi Yunlong, > > On Sat, Feb 24, 2018 at 05:14:58PM +0800, 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 922ec355f86365388203672119b5bca346a45085 ("f2fs crypto: avoid >> unneeded memory allocation when {en/de}crypting symlink") >> commit e06f86e61d7a67fe6e826010f57aa39c674f4b1b ("f2fs crypto: avoid >> unneeded memory allocation in ->readdir") >> >> reverts the codes, which causes panic again in arm, so let's add the old >> patch again. >> >> Signed-off-by: Yunlong Song >> --- >> fs/f2fs/dir.c | 7 +++++++ >> fs/f2fs/namei.c | 10 +++++++++- >> 2 files changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c >> index f00b5ed..c0cf3e7b 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 = kmalloc(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, >> &de_name, fstr); >> + kfree(de_name.name); >> if (err) >> return err; >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index c4c94c7..2cb70c1 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -1170,8 +1170,13 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >> >> /* Symlink is encrypted */ >> sd = (struct fscrypt_symlink_data *)caddr; >> - cstr.name = sd->encrypted_path; >> cstr.len = le16_to_cpu(sd->len); >> + cstr.name = kmalloc(cstr.len, GFP_NOFS); >> + if (!cstr.name) { >> + res = -ENOMEM; >> + goto errout; >> + } >> + memcpy(cstr.name, sd->encrypted_path, cstr.len); >> >> /* this is broken symlink case */ >> if (unlikely(cstr.len == 0)) { >> @@ -1198,6 +1203,8 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >> goto errout; >> } >> >> + kfree(cstr.name); >> + >> paddr = pstr.name; >> >> /* Null-terminate the name */ >> @@ -1207,6 +1214,7 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >> set_delayed_call(done, kfree_link, paddr); >> return paddr; >> errout: >> + kfree(cstr.name); >> fscrypt_fname_free_buffer(&pstr); >> put_page(cpage); >> return ERR_PTR(res); >> -- >> 1.8.5.2 >> > > The pagecache for symlinks in f2fs already uses lowmem only, so the change to > ->get_link() isn't needed. Also that part of the patch doesn't apply to > upstream. > > For directories we may need your fix. Note: kmalloc + memcpy should be replaced > with kmemdup. But alternatively, directory pages could be restricted to lowmem > which would match ext4. > > Eric > > . > -- Thanks, Yunlong Song