Re: [PATCH] drop the deprecated malloc/free hooks in hurd/mach-defpager

[Thomas Schwinge]

Hi!

On Tue, 29 Dec 2015 23:09:54 +0100, Flavio Cruz <flaviocruz@gmail.com> wrote:
> mach-defpager: Drop the deprecated malloc/free hooks.

(After approval by Samuel, this became commit
8c49801c8f7e3f800cabedf8fca8ccec3cf35a22.)

> * mach-defpager/kalloc.c: Define malloc and free directly since those are weak
> symbols.

The idea here is to just change the mechanism (override glibc's weak
symbols instead of using glibc's deprecated malloc hooks), while not
changing any semantics, right?

From a quick look, I have some doubts about this generally (so, not only
related to your change).

> --- a/mach-defpager/kalloc.c
> +++ b/mach-defpager/kalloc.c

> -static void *
> -malloc_hook (size_t size, const void *caller)
> +void *
> +malloc (size_t size)
>  {
>    return (void *) kalloc ((vm_size_t) size);
>  }
>  
> -static void
> -free_hook (void *ptr, const void *caller)
> +void
> +free (void *ptr)
>  {
>    /* Just ignore harmless attempts at cleanliness.  */
>    /* panic("free not implemented"); */

So, here we now only provide the malloc and free symbols.  What happens
if, for example, calloc, realloc, posix_memalign et al. are called?

For example, looking at [glibc]/malloc/malloc.c:__libc_calloc (weak alias
for calloc).  Before your change, it called mach-defpager's malloc_hook,
and then zeroed that region.  After your change, it is implemented using
the real glibc calloc -- which actually does change the semantics in an
unintended way?

Backing up one step: what is actually being done here in mach-defpager?
Again from just a quick look, is the idea to use wired memory for all of
mach-defpager's process memory?

So, for example, if now some mach-defpager code calls calloc, it will now
get non-wired "glibc" memory instead of wired "kalloc" memory.  The
problem here is not mach-defpager's own code (which we can easily
verify/control), but the problems is the libraries it links against,
which currently are: libihash, libpthread, glibc itself, and any
dependencies these may have.  Can we be sure these are not internally
using calloc, for example?

Now, this is not a new problem that you introduced ;-) -- previously we
also didn't provide malloc's memalign and realloc hooks, and realloc is
used quite commonly, I suppose?

If my theories are correct, do we need to use a different mechanism to
get the whole mach-defpager process into wired memory?


Grüße
 Thomas

signature.asc
Description: PGP signature