From: Zhihao Cheng <chengzhihao1@huawei.com> To: <richard@nod.at>, <ebiggers@google.com>, <terrelln@fb.com> Cc: <linux-fscrypt@vger.kernel.org>, <linux-mtd@lists.infradead.org> Subject: [PATCH v3 2/2] ubifs: ubifs_symlink: Fix memleak of inode->i_link in error path Date: Mon, 8 Jan 2024 10:41:05 +0800 [thread overview] Message-ID: <20240108024105.194516-3-chengzhihao1@huawei.com> (raw) In-Reply-To: <20240108024105.194516-1-chengzhihao1@huawei.com> For error handling path in ubifs_symlink(), inode will be marked as bad first, then iput() is invoked. If inode->i_link is initialized by fscrypt_encrypt_symlink() in encryption scenario, inode->i_link won't be freed by callchain ubifs_free_inode -> fscrypt_free_inode in error handling path, because make_bad_inode() has changed 'inode->i_mode' as 'S_IFREG'. Following kmemleak is easy to be reproduced by injecting error in ubifs_jnl_update() when doing symlink in encryption scenario: unreferenced object 0xffff888103da3d98 (size 8): comm "ln", pid 1692, jiffies 4294914701 (age 12.045s) backtrace: kmemdup+0x32/0x70 __fscrypt_encrypt_symlink+0xed/0x1c0 ubifs_symlink+0x210/0x300 [ubifs] vfs_symlink+0x216/0x360 do_symlinkat+0x11a/0x190 do_syscall_64+0x3b/0xe0 There are two ways fixing it: 1. Remove make_bad_inode() in error handling path. We can do that because ubifs_evict_inode() will do same processes for good symlink inode and bad symlink inode, for inode->i_nlink checking is before is_bad_inode(). 2. Free inode->i_link before marking inode bad. Method 2 is picked, it has less influence, personally, I think. Cc: stable@vger.kernel.org Fixes: 2c58d548f570 ("fscrypt: cache decrypted symlink target in ->i_link") Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Suggested-by: Eric Biggers <ebiggers@kernel.org> Reviewed-by: Eric Biggers <ebiggers@google.com> --- fs/ubifs/dir.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 3b13c648d490..e413a9cf8ee3 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1234,6 +1234,8 @@ static int ubifs_symlink(struct mnt_idmap *idmap, struct inode *dir, dir_ui->ui_size = dir->i_size; mutex_unlock(&dir_ui->ui_mutex); out_inode: + /* Free inode->i_link before inode is marked as bad. */ + fscrypt_free_inode(inode); make_bad_inode(inode); iput(inode); out_fname: -- 2.39.2
WARNING: multiple messages have this Message-ID (diff)
From: Zhihao Cheng <chengzhihao1@huawei.com> To: <richard@nod.at>, <ebiggers@google.com>, <terrelln@fb.com> Cc: <linux-fscrypt@vger.kernel.org>, <linux-mtd@lists.infradead.org> Subject: [PATCH v3 2/2] ubifs: ubifs_symlink: Fix memleak of inode->i_link in error path Date: Mon, 8 Jan 2024 10:41:05 +0800 [thread overview] Message-ID: <20240108024105.194516-3-chengzhihao1@huawei.com> (raw) In-Reply-To: <20240108024105.194516-1-chengzhihao1@huawei.com> For error handling path in ubifs_symlink(), inode will be marked as bad first, then iput() is invoked. If inode->i_link is initialized by fscrypt_encrypt_symlink() in encryption scenario, inode->i_link won't be freed by callchain ubifs_free_inode -> fscrypt_free_inode in error handling path, because make_bad_inode() has changed 'inode->i_mode' as 'S_IFREG'. Following kmemleak is easy to be reproduced by injecting error in ubifs_jnl_update() when doing symlink in encryption scenario: unreferenced object 0xffff888103da3d98 (size 8): comm "ln", pid 1692, jiffies 4294914701 (age 12.045s) backtrace: kmemdup+0x32/0x70 __fscrypt_encrypt_symlink+0xed/0x1c0 ubifs_symlink+0x210/0x300 [ubifs] vfs_symlink+0x216/0x360 do_symlinkat+0x11a/0x190 do_syscall_64+0x3b/0xe0 There are two ways fixing it: 1. Remove make_bad_inode() in error handling path. We can do that because ubifs_evict_inode() will do same processes for good symlink inode and bad symlink inode, for inode->i_nlink checking is before is_bad_inode(). 2. Free inode->i_link before marking inode bad. Method 2 is picked, it has less influence, personally, I think. Cc: stable@vger.kernel.org Fixes: 2c58d548f570 ("fscrypt: cache decrypted symlink target in ->i_link") Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Suggested-by: Eric Biggers <ebiggers@kernel.org> Reviewed-by: Eric Biggers <ebiggers@google.com> --- fs/ubifs/dir.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 3b13c648d490..e413a9cf8ee3 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1234,6 +1234,8 @@ static int ubifs_symlink(struct mnt_idmap *idmap, struct inode *dir, dir_ui->ui_size = dir->i_size; mutex_unlock(&dir_ui->ui_mutex); out_inode: + /* Free inode->i_link before inode is marked as bad. */ + fscrypt_free_inode(inode); make_bad_inode(inode); iput(inode); out_fname: -- 2.39.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2024-01-08 2:44 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-01-08 2:41 [PATCH v3 0/2] ubifs: Fix two kmemleaks in error path Zhihao Cheng 2024-01-08 2:41 ` Zhihao Cheng 2024-01-08 2:41 ` [PATCH v3 1/2] ubifs: dbg_check_idx_size: Fix kmemleak if loading znode failed Zhihao Cheng 2024-01-08 2:41 ` Zhihao Cheng 2024-01-08 2:41 ` Zhihao Cheng [this message] 2024-01-08 2:41 ` [PATCH v3 2/2] ubifs: ubifs_symlink: Fix memleak of inode->i_link in error path Zhihao Cheng
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20240108024105.194516-3-chengzhihao1@huawei.com \ --to=chengzhihao1@huawei.com \ --cc=ebiggers@google.com \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=richard@nod.at \ --cc=terrelln@fb.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.