bug#58732: installer: finalizers & device destroy segfault

[Ludovic Courtès]

Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> skribis:

> I made some progress on that one. I think, this is what's going on:
>
> 1. Two new PedDevice A and B are malloc'ed by the libparted when opening
> the installer partitioning page.
>
> 2. They are added to the %devices weak hash table by pointer->device!
> and their respective finalizers are registered.
>
> 3. The partitioning ends and A goes out of scope. It is eventually
> removed from %devices but it does not mean its finalizer will be run
> immediately.
>
> 4. The partitioning is restarted using the installer menu. B is still in
> the %devices hash table. However, A is now gone and is added again to
> the %devices hash table by the pointer->device! procedure. Another
> finalizer is registered for A.
>
> That's because set-pointer-finalizer! does not *set* a finalizer it
> *adds* one.

Oh, I think I see what you mean.  You’re right about
‘set-pointer-finalizer!’ adding a finalizer, but I don’t think that’s
what’s happening here.

Finalizers are set on pointer objects, so they’re invoked when the
pointer object goes out of scope.  But:

  (eq? (make-pointer 123) (make-pointer 123))
  => #f

So a possible mistake is to add one finalizer on each pointer object and
have several pointer objects aliasing the same C object; that’s how you
can get the same “free” function called several times on the same C
object.

> 5. The partitioning ends and both A and B goes out of scope. They are
> removed from %devices and their finalizers are called. The A finalizer
> is called twice resulting in a double free.
>
> This race condition is created by the fact that there is a time window
> where the device is removed from the %devices hash table but its
> finalizer is not immediately called.

What if we create an extra hashv table that maps pointer values
(integers) to pointer objects?

  (define %pointers (make-hash-table))

  (define (canonical-pointer ptr)
    (or (hashv-ref %pointers (pointer-address ptr))
        (begin
          (hashv-set! %pointers (pointer-address ptr) ptr)
          ptr)))

This is kinda terrible but it would allow us to test the above
hypothesis.

Thanks,
Ludo’.