From: Eric Biggers <ebiggers@kernel.org> To: linux-ext4@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, stable@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>, Daniel Rosenberg <drosen@google.com>, Gabriel Krisman Bertazi <krisman@collabora.co.uk> Subject: [PATCH] ext4: avoid utf8_strncasecmp() with unstable name Date: Fri, 29 May 2020 23:02:16 -0700 [thread overview] Message-ID: <20200530060216.221456-1-ebiggers@kernel.org> (raw) From: Eric Biggers <ebiggers@google.com> If the dentry name passed to ->d_compare() fits in dentry::d_iname, then it may be concurrently modified by a rename. This can cause undefined behavior (possibly out-of-bounds memory accesses or crashes) in utf8_strncasecmp(), since fs/unicode/ isn't written to handle strings that may be concurrently modified. Fix this by first copying the filename to a stack buffer if needed. This way we get a stable snapshot of the filename. Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups") Cc: <stable@vger.kernel.org> # v5.2+ Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Daniel Rosenberg <drosen@google.com> Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk> Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/ext4/dir.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c index c654205f648dd..19aef8328bb18 100644 --- a/fs/ext4/dir.c +++ b/fs/ext4/dir.c @@ -675,6 +675,7 @@ static int ext4_d_compare(const struct dentry *dentry, unsigned int len, struct qstr qstr = {.name = str, .len = len }; const struct dentry *parent = READ_ONCE(dentry->d_parent); const struct inode *inode = READ_ONCE(parent->d_inode); + char strbuf[DNAME_INLINE_LEN]; if (!inode || !IS_CASEFOLDED(inode) || !EXT4_SB(inode->i_sb)->s_encoding) { @@ -683,6 +684,22 @@ static int ext4_d_compare(const struct dentry *dentry, unsigned int len, return memcmp(str, name->name, len); } + /* + * If the dentry name is stored in-line, then it may be concurrently + * modified by a rename. If this happens, the VFS will eventually retry + * the lookup, so it doesn't matter what ->d_compare() returns. + * However, it's unsafe to call utf8_strncasecmp() with an unstable + * string. Therefore, we have to copy the name into a temporary buffer. + */ + if (len <= DNAME_INLINE_LEN - 1) { + unsigned int i; + + for (i = 0; i < len; i++) + strbuf[i] = READ_ONCE(str[i]); + strbuf[len] = 0; + qstr.name = strbuf; + } + return ext4_ci_compare(inode, name, &qstr, false); } -- 2.26.2
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: linux-ext4@vger.kernel.org Cc: Daniel Rosenberg <drosen@google.com>, stable@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Al Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org, Gabriel Krisman Bertazi <krisman@collabora.co.uk> Subject: [f2fs-dev] [PATCH] ext4: avoid utf8_strncasecmp() with unstable name Date: Fri, 29 May 2020 23:02:16 -0700 [thread overview] Message-ID: <20200530060216.221456-1-ebiggers@kernel.org> (raw) From: Eric Biggers <ebiggers@google.com> If the dentry name passed to ->d_compare() fits in dentry::d_iname, then it may be concurrently modified by a rename. This can cause undefined behavior (possibly out-of-bounds memory accesses or crashes) in utf8_strncasecmp(), since fs/unicode/ isn't written to handle strings that may be concurrently modified. Fix this by first copying the filename to a stack buffer if needed. This way we get a stable snapshot of the filename. Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups") Cc: <stable@vger.kernel.org> # v5.2+ Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Daniel Rosenberg <drosen@google.com> Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk> Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/ext4/dir.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c index c654205f648dd..19aef8328bb18 100644 --- a/fs/ext4/dir.c +++ b/fs/ext4/dir.c @@ -675,6 +675,7 @@ static int ext4_d_compare(const struct dentry *dentry, unsigned int len, struct qstr qstr = {.name = str, .len = len }; const struct dentry *parent = READ_ONCE(dentry->d_parent); const struct inode *inode = READ_ONCE(parent->d_inode); + char strbuf[DNAME_INLINE_LEN]; if (!inode || !IS_CASEFOLDED(inode) || !EXT4_SB(inode->i_sb)->s_encoding) { @@ -683,6 +684,22 @@ static int ext4_d_compare(const struct dentry *dentry, unsigned int len, return memcmp(str, name->name, len); } + /* + * If the dentry name is stored in-line, then it may be concurrently + * modified by a rename. If this happens, the VFS will eventually retry + * the lookup, so it doesn't matter what ->d_compare() returns. + * However, it's unsafe to call utf8_strncasecmp() with an unstable + * string. Therefore, we have to copy the name into a temporary buffer. + */ + if (len <= DNAME_INLINE_LEN - 1) { + unsigned int i; + + for (i = 0; i < len; i++) + strbuf[i] = READ_ONCE(str[i]); + strbuf[len] = 0; + qstr.name = strbuf; + } + return ext4_ci_compare(inode, name, &qstr, false); } -- 2.26.2 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next reply other threads:[~2020-05-30 6:03 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-30 6:02 Eric Biggers [this message] 2020-05-30 6:02 ` [f2fs-dev] [PATCH] ext4: avoid utf8_strncasecmp() with unstable name Eric Biggers 2020-05-30 6:17 ` Gabriel Krisman Bertazi 2020-05-30 6:17 ` [f2fs-dev] " Gabriel Krisman Bertazi 2020-05-30 6:44 ` Eric Biggers 2020-05-30 6:44 ` [f2fs-dev] " Eric Biggers 2020-05-30 6:52 ` Gabriel Krisman Bertazi 2020-05-30 17:18 ` Matthew Wilcox 2020-05-30 17:18 ` [f2fs-dev] " Matthew Wilcox 2020-05-30 17:35 ` Eric Biggers 2020-05-30 17:35 ` [f2fs-dev] " Eric Biggers 2020-05-30 17:59 ` Al Viro 2020-05-30 17:59 ` [f2fs-dev] " Al Viro 2020-06-01 6:45 ` Eric Biggers 2020-06-01 6:45 ` [f2fs-dev] " Eric Biggers 2020-05-30 20:41 ` Matthew Wilcox 2020-05-30 20:41 ` [f2fs-dev] " Matthew Wilcox 2020-06-01 7:05 ` Eric Biggers 2020-06-01 7:05 ` [f2fs-dev] " Eric Biggers
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=20200530060216.221456-1-ebiggers@kernel.org \ --to=ebiggers@kernel.org \ --cc=drosen@google.com \ --cc=krisman@collabora.co.uk \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fsdevel@vger.kernel.org \ --cc=stable@vger.kernel.org \ --cc=viro@zeniv.linux.org.uk \ /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.