[PATCH v2] chardev/tcp: fix error message double free error

lichun posted 1 patch 3 years, 10 months ago
Test FreeBSD passed
Test asan passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200621213017.17978-1-lichun@ruijie.com.cn
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
chardev/char-socket.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v2] chardev/tcp: fix error message double free error
Posted by lichun 3 years, 10 months ago
Signed-off-by: lichun <lichun@ruijie.com.cn>
---
 chardev/char-socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index afebeec5c3..569d54c144 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
                           "Unable to connect character device %s: ",
                           chr->label);
         s->connect_err_reported = true;
+    } else {
+        error_free(err);
     }
     qemu_chr_socket_restart_timer(chr);
 }
@@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
     if (qio_task_propagate_error(task, &err)) {
         tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
         check_report_connect_error(chr, err);
-        error_free(err);
         goto cleanup;
     }
 
-- 
2.18.4


Re: [PATCH v2] chardev/tcp: fix error message double free error
Posted by Markus Armbruster 3 years, 10 months ago
lichun <lichun@ruijie.com.cn> writes:

> Signed-off-by: lichun <lichun@ruijie.com.cn>
> ---
>  chardev/char-socket.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index afebeec5c3..569d54c144 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
>                            "Unable to connect character device %s: ",
>                            chr->label);
>          s->connect_err_reported = true;
> +    } else {
> +        error_free(err);
>      }
>      qemu_chr_socket_restart_timer(chr);
>  }
> @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
>      if (qio_task_propagate_error(task, &err)) {
>          tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>          check_report_connect_error(chr, err);
> -        error_free(err);
>          goto cleanup;
>      }

Since my "Error handling fixes & cleanups" series fixes similar errors.
I'm happy to merge this patch along with it.  Up to Marc-André.


Re: [PATCH v2] chardev/tcp: fix error message double free error
Posted by Marc-André Lureau 3 years, 10 months ago
Hi

On Thu, Jun 25, 2020 at 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> lichun <lichun@ruijie.com.cn> writes:
>
> > Signed-off-by: lichun <lichun@ruijie.com.cn>
> > ---
> >  chardev/char-socket.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > index afebeec5c3..569d54c144 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
> >                            "Unable to connect character device %s: ",
> >                            chr->label);
> >          s->connect_err_reported = true;
> > +    } else {
> > +        error_free(err);
> >      }
> >      qemu_chr_socket_restart_timer(chr);
> >  }
> > @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
> >      if (qio_task_propagate_error(task, &err)) {
> >          tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
> >          check_report_connect_error(chr, err);
> > -        error_free(err);
> >          goto cleanup;
> >      }
>
> Since my "Error handling fixes & cleanups" series fixes similar errors.
> I'm happy to merge this patch along with it.  Up to Marc-André.

That would be great, thanks!


Re: [PATCH v2] chardev/tcp: fix error message double free error
Posted by Markus Armbruster 3 years, 10 months ago
lichun <lichun@ruijie.com.cn> writes:

> Signed-off-by: lichun <lichun@ruijie.com.cn>
> ---
>  chardev/char-socket.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index afebeec5c3..569d54c144 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
>                            "Unable to connect character device %s: ",
>                            chr->label);
>          s->connect_err_reported = true;
> +    } else {
> +        error_free(err);
>      }
>      qemu_chr_socket_restart_timer(chr);
>  }
> @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
>      if (qio_task_propagate_error(task, &err)) {
>          tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>          check_report_connect_error(chr, err);
> -        error_free(err);
>          goto cleanup;
>      }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

and queued, thanks!


Re: [PATCH v2] chardev/tcp: fix error message double free error
Posted by Paolo Bonzini 3 years, 10 months ago
On 01/07/20 09:06, Markus Armbruster wrote:
> lichun <lichun@ruijie.com.cn> writes:
> 
>> Signed-off-by: lichun <lichun@ruijie.com.cn>
>> ---
>>  chardev/char-socket.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index afebeec5c3..569d54c144 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
>>                            "Unable to connect character device %s: ",
>>                            chr->label);
>>          s->connect_err_reported = true;
>> +    } else {
>> +        error_free(err);
>>      }
>>      qemu_chr_socket_restart_timer(chr);
>>  }
>> @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
>>      if (qio_task_propagate_error(task, &err)) {
>>          tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>>          check_report_connect_error(chr, err);
>> -        error_free(err);
>>          goto cleanup;
>>      }
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> and queued, thanks!
> 

Can you please add a note to the commit message?

    Errors are already freed by error_report_err, so we only need to call
    error_free when that function is not called.
    
and Cc qemu-stable?  Or I can queue it too since it's chardev stuff.


Re: [PATCH v2] chardev/tcp: fix error message double free error
Posted by Markus Armbruster 3 years, 10 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 01/07/20 09:06, Markus Armbruster wrote:
>> lichun <lichun@ruijie.com.cn> writes:
>> 
>>> Signed-off-by: lichun <lichun@ruijie.com.cn>
>>> ---
>>>  chardev/char-socket.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>>> index afebeec5c3..569d54c144 100644
>>> --- a/chardev/char-socket.c
>>> +++ b/chardev/char-socket.c
>>> @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
>>>                            "Unable to connect character device %s: ",
>>>                            chr->label);
>>>          s->connect_err_reported = true;
>>> +    } else {
>>> +        error_free(err);
>>>      }
>>>      qemu_chr_socket_restart_timer(chr);
>>>  }
>>> @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
>>>      if (qio_task_propagate_error(task, &err)) {
>>>          tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>>>          check_report_connect_error(chr, err);
>>> -        error_free(err);
>>>          goto cleanup;
>>>      }
>> 
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> 
>> and queued, thanks!
>> 
>
> Can you please add a note to the commit message?
>
>     Errors are already freed by error_report_err, so we only need to call
>     error_free when that function is not called.
>     
> and Cc qemu-stable?  Or I can queue it too since it's chardev stuff.

Done in my tree, expect PR later today.