Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230503091340.4627057a@inria.fr>
Date: Wed, 3 May 2023 09:13:40 +0200
From: Jₑₙₛ Gustedt <jens.gustedt@...ia.fr>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: patches for C23

Rich,

on Tue, 2 May 2023 19:20:09 -0400 you (Rich Felker <dalias@...c.org>)
wrote:

> On Tue, May 02, 2023 at 03:59:03PM +0200, Jₑₙₛ Gustedt wrote:
> > on Tue, 2 May 2023 08:57:40 +0200 you (Jₑₙₛ Gustedt
> > <jens.gustedt@...ia.fr>) wrote:
> >   
> > > I'll also setup a git repository for those who would be willing to
> > > test the whole. Just be aware that is really testing and review,
> > > not yet ready for direct inclusion. So probably this will be
> > > rebased several times.  
> > 
> > So this can now be found here
> > 
> >    https://icube-forge.unistra.fr/icps/musl/
> > 
> > with my additional branch called "c23". I also put on tags for what
> > I think might be good groups to treat together. An overview should
> > be accessible here
> > 
> >            https://icube-forge.unistra.fr/icps/musl/-/network/c23?ref_type=heads
> > 
> > Let me know if you have any problems in accessing this.
> > 
> > I will then post the patches on the ML later, probably need some
> > time for that to do it right.  
> 
> One quick find, in
> https://icube-forge.unistra.fr/icps/musl/-/commit/3a2b83bf32d7c94f1bf0b2b2fd6ba8b6bf980d99
> 
> -				np = strtoul(r+9, &z, 10);
> +				np = strtoul(r+9, (char**)&z, 10);
> 
> is UB.

I think it the situation is more subtle than that. If this were
application C code the implementation of `strtoul` would provoque UB
under certain circumstances. And this UB would be happening in line 16
of strtol.c, not at the calling side. Here at the calling side, we
only have a pointer cast, which as such is well-defined because the
two pointer types have same representation and alignment.

Spinning that further, the code would then be UB as written before
(with an unqualified `z`) because the call to `strtoul` then stores a
`char const*` value into a `char*` object. By providing a declaration
of `z` as `char const*`, the store in fact is now valid. So with that
application side notion of UB, the proposed patch changes the code
from UB to well-defined.

But since we are the C library implementation nothing of that is UB,
because the C standard enforces that the `char const*` value is stored
in a `char*` object.

So that cast as written above only calls out a special situation.

> Accessing a const char * as char *. I would prefer in general
> we just #undef any of the const-stuff-tg macros in files that use
> them, or just have src/include/string.h always do that. (Not really
> needed since musl source is written in c99 not c23, but it would be
> nice to have it also compile with c11 and c23 compilers, so I think
> the #undef is useful.)

I am not sure that I understand how you think that should work, we
have to provide these tg macros to our users, don't we?

In any case, I prefer to mark such positions explicitly with something
like `(strchr)(r, '\n')` as in line 222 of the code that you are
refering to. All of this is marked as obsolescent in 7.26.5.1

         If a macro definition of any of these generic functions is
         suppressed to access an actual function, the external
         declaration with the corresponding concrete type is
         visible.381)

         381) This is an obsolescent feature.

Jₑₙₛ

-- 
:: ICube :::::::::::::::::::::::::::::: deputy director ::
:: Université de Strasbourg :::::::::::::::::::::: ICPS ::
:: INRIA Nancy Grand Est :::::::::::::::::::::::: Camus ::
:: :::::::::::::::::::::::::::::::::::: ☎ +33 368854536 ::
:: https://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

Content of type "application/pgp-signature" skipped

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.