[PATCH 2/2] malloc: introduce ifuncs for malloc functions

[DJ Delorie]

Yury Khrustalev <yury.khrustalev@arm.com> writes:
>> > @@ -3142,7 +3143,7 @@ tcache_double_free_verify (tcache_entry *e)
>> >       or user data that happens to match the key.  Since we are not sure,
>> >       clear the key and retry freeing it.  */
>> >    e->key = 0;
>> > -  __libc_free (e);
>> > +  __libc_free_core (e);
>> >  }
>> 
>> This is only called from within __libc_free_core itself anyway, and only
>> for the same block, so any ifunc wrapper would have already had its
>> chance at this chunk.  Thus, not calling the ifunc wrapper again seems
>> correct to me.
>
> Is this "OK" or "Not OK"? :)

Sorry, OK.

> This is the fragile part of any abstractions around malloc
> (ifunc-based or otherwise).

Yeah, that's why I put my thoughts in writing, in case they were faulty.

> Off-topic, but I think that all internal _core functions should operate on
> something like

In practice, most operate on a CHUNK* (mchunkptr) but we don't have a
special type for pointers to the user data part.

>> I wonder, though... we current do tagging operations inside malloc
>> internals, like splitting chunks.  How will the ifunc interface handle
>> these?
>
> That's left for the next patch series, stay tuned! Core parts of malloc
> should not be aware of any tagging or whatnot. I'm working on it now.

I will find it interesting how you do that, because we intentionally
change the tagging when we do that to avoid certain types of attack
vectors.

>> It would be nice if there were some hint as to what these arguments are,
>> or are used for.  Ifuncs are complicated enough without obfuscating this
>> information.
>
> That's why I kept them as part of macro arguments. Perhaps, we should make
> it even more explicit. I'll think about it. Maybe at least argument types
> should be here too. As for the names, I think they have to be pretty generic.

Sorry, I meant the names should give a hint as to what the arguments
*are*, or a comment that says "these will be passed the following
data...".

Just saying "arg0" and "argv[]" tells the reader nothing about what
those values *mean* or where they come from.