From: "Valentin Vidić" <vvidic@valentin-vidic.from.hr>
To: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org
Subject: Re: [Ocfs2-devel] [PATCH] ocfs2: mount fails with buffer overflow in strlen
Date: Wed, 29 Sep 2021 08:24:34 +0200 [thread overview]
Message-ID: <20210929062434.GN28341@valentin-vidic.from.hr> (raw)
In-Reply-To: <212f878e-1bbe-347c-ba43-e4ffb9b4afbe@linux.alibaba.com>
On Wed, Sep 29, 2021 at 10:38:59AM +0800, Joseph Qi wrote:
> Okay, you are right, strlen(src) is indeed wrong here.
>
> But please note that in strlcpy():
> size_t ret = strlen(src);
> if (size) {
> size_t len = (ret >= size) ? size - 1 : ret;
> memcpy(dest, src, len);
> dest[len] = '\0';
> }
>
> Take ci_stack "o2cb" for example, strlen("o2cb") may return wrong if the
> coming byte is not null, say it is 10.
> The input size is 5, so len will finally be 4.
> So dest is still correct ending with null byte. No overflow happens.
> So the problem here is the wrong return value, but it is discarded in
> ocfs2_initialize_super().
strlcpy starts with a call to strlen(src) and this is where the read overflow
happens. If the kernel is compiled with CONFIG_FORTIFY_SOURCE this gets
executed instead (include/linux/fortify-string.h):
__FORTIFY_INLINE __kernel_size_t strlen(const char *p)
{
__kernel_size_t ret;
size_t p_size = __builtin_object_size(p, 1);
/* Work around gcc excess stack consumption issue */
if (p_size == (size_t)-1 ||
(__builtin_constant_p(p[p_size - 1]) && p[p_size - 1] == '\0'))
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
fortify_panic(__func__);
return ret;
}
So while strlcpy did work before this fortify check, it is probably not the
best option anymore due to the missing null terminator in the source.
--
Valentin
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
next prev parent reply other threads:[~2021-09-29 18:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-27 15:44 [Ocfs2-devel] [PATCH] ocfs2: mount fails with buffer overflow in strlen Valentin Vidic
2021-09-28 12:05 ` Joseph Qi
2021-09-28 13:14 ` Valentin Vidić
2021-09-29 2:38 ` Joseph Qi
2021-09-29 6:24 ` Valentin Vidić [this message]
2021-09-29 9:12 ` Joseph Qi
2021-09-29 18:06 ` [Ocfs2-devel] [PATCH v2] " Valentin Vidic
2021-09-30 1:54 ` Joseph Qi
2021-10-08 10:46 ` Gang He
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=20210929062434.GN28341@valentin-vidic.from.hr \
--to=vvidic@valentin-vidic.from.hr \
--cc=joseph.qi@linux.alibaba.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ocfs2-devel@oss.oracle.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).