[Qemu-devel] Re: [PATCH 05/18] vl.c: add deleted flag for deleting the handler.

[Juan Quintela]

Yoshiaki Tamura <address@hidden> wrote:
> Make deleting handlers robust against deletion of any elements in a
> handler by using a deleted flag like in file descriptors.
>
> Signed-off-by: Yoshiaki Tamura <address@hidden>
> ---
>  vl.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index b436952..4e263c3 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1158,6 +1158,7 @@ static void nographic_update(void *opaque)
>  struct vm_change_state_entry {
>      VMChangeStateHandler *cb;
>      void *opaque;
> +    int deleted;
>      QLIST_ENTRY (vm_change_state_entry) entries;
>  };
>  
> @@ -1178,8 +1179,7 @@ VMChangeStateEntry 
> *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>  
>  void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
>  {
> -    QLIST_REMOVE (e, entries);
> -    qemu_free (e);
> +    e->deleted = 1;
>  }
>  
>  void vm_state_notify(int running, int reason)
> @@ -1188,8 +1188,13 @@ void vm_state_notify(int running, int reason)
>  
>      trace_vm_state_notify(running, reason);
>  
> -    for (e = vm_change_state_head.lh_first; e; e = e->entries.le_next) {
> -        e->cb(e->opaque, running, reason);

this needs to become:

> +    QLIST_FOREACH(e, &vm_change_state_head, entries) {
> +        if (e->deleted) {
> +            QLIST_REMOVE(e, entries);
> +            qemu_free(e);
> +        } else {
> +            e->cb(e->opaque, running, reason);
> +        }

   VMChangeState_entry *next;

   QLIST_FOREACH_SAFE(e, &vm_change_state_head, entries, next) {
      .....

  Otherwise you are accessing "e" after qemu_free and being put out of
  the list.

Later, Juan.