Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0666848b-fea6-a692-5a7f-6b0daa829f09@ispras.ru>
Date: Sat, 27 Jan 2024 15:58:02 +0300 (MSK)
From: Alexander Monakov <amonakov@...ras.ru>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
cc: Andy Caldwell <andycaldwell@...rosoft.com>
Subject: Re: RE: [EXTERNAL] Re: [PATCH] fix avoidable segfault
 in catclose


On Sat, 27 Jan 2024, Szabolcs Nagy wrote:

> > Yes, this - the details aren't particularly interesting but the key is that "invoke UB"
> > is not the same as "crash/trap".  I'm also contrasting this to the comments in the
> > glibc wiki and Markus's synopsis (from the earlier email) that "it has been musl policy
> > to crash on invalid args since the beginning" - in the face of UB, musl (and presumably
> > also glibc) _doesn't_ crash/trap, nor does it "fail early and catastrophically" it
> > instead "propagates the UB".  In debug builds these are often equivalent, but the
> > specific path to UB might not be seen in a debug build, and only be seen in production
> > where the non-locality of UB effects are at their worst.
> 
> i think you are still looking at this the wrong way:
> 
> - the original code has ub.
> - so anything can happen.
> - whatever libc does, still anything can happen.
> - adding a check p==-1 in libc does not change anything.
> (the ub already happens in the caller. a compiler can even remove the
> call since it can know about catclose semantics.)
> 
> given these facts on the theoretical level, we can look pragmatically at
> the actual transformations a compiler would likely do and we find that
> an invalid NULL+n dereference in practice is almost surely an immediate
> crash (on linux with dynamic linking or static linking without lto this
> is not only likely but actually guaranteed by existing toolchains) which
> is the best possible outcome for debugging, meanwhile an extra check in
> libc is worse: the code continues and misbehaves somewhere else.

I don't think this follows. I believe the suggestion was to have

    if (catd == (nl_catd)-1) a_crash();

which is the opposite of "continuing and misbehaving".

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.