From: Rich Felker <email@example.com>
To: Joakim Sindholt <firstname.lastname@example.org>
Cc: email@example.com, James R T <firstname.lastname@example.org>
Subject: Re: [musl] [PATCH] Add a safe dequeue integrity check for mallocng
Date: Thu, 14 Sep 2023 08:18:35 -0400 [thread overview]
Message-ID: <20230914121834.GE4163@brightrain.aerifal.cx> (raw)
On Thu, Sep 14, 2023 at 11:23:08AM +0200, Joakim Sindholt wrote:
> On Thu, 14 Sep 2023 13:13:23 +0800, James R T <email@example.com> wrote:
> > On Sat, Sep 9, 2023 at 8:48 AM Rich Felker <firstname.lastname@example.org> wrote:
> > >
> > > This could and should be written with the assert macro, like all the
> > > other safety assertions in mallocng, not pulling in stdio and abort.
> > Understood. I was not able to find an assert with `predict_false` for
> > the condition. Should I add one assert function with `predict_false`
> > in `include/assert.h` or `src/exit/assert.c` or simply use the regular
> > assert?
> It's a little confusing but assert() in mallocng is not real assert():
> The issue is that if memory is under control of an attacker then doing
> anything at all, especially running the stdio machinery, is unsafe. To
> that end musl uses a_crash() here which expands to a minimal set of
> instructions to crash the process:
Yes. mallocng is written such that you could use the normal assert
with it, but presently it's just expanding to a_crash(). At some point
this might be revamped to crash with a message string in a particular
register or argument slot or something so that you get a bit more
meaningful information if looking at it in a debugger. And indeed, the
reason not to do any message printing, etc. is that you're running in
a known-compromised process state where any further complex execution
is unsafe (e.g. if the out-of-band malloc metadata was clobbered, the
function pointers in stderr might also have been clobbered, since the
latter are *easier to reach* than the OOB metadata).
> Furthermore, musl doesn't use any of thosed tagged branch tricks and I
> personally doubt it would make any difference.
Yes, the only reason libm.h has them is because nsz is using the code
in other environments that want them, and it made sense to avoid
gratuitous differences. We don't generally use those in musl. If the
compiler isn't generating good code and puts the failure path as a hot
path, we probably should explore whether the compiler is missing that
it's a does-not-return thing (which should always be treated as cold).
But indeed I doubt it makes a difference.
next prev parent reply other threads:[~2023-09-14 12:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-08 17:49 James Raphael Tiovalen
2023-09-09 0:48 ` Rich Felker
2023-09-14 5:13 ` James R T
2023-09-14 9:23 ` Joakim Sindholt
2023-09-14 12:18 ` Rich Felker [this message]
2023-09-16 6:53 ` James R T
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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.
Code repositories for project(s) associated with this public inbox
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).