[PATCH] remote: handle partial data transmission

Oleg Vasilev posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230726074722.40979-1-oleg.vasilev@virtuozzo.com
src/remote/remote_daemon_stream.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH] remote: handle partial data transmission
Posted by Oleg Vasilev 9 months, 2 weeks ago
A new bug was introduced as a part of use-after-free fix below:

    commit 411cbe7199ce533ae5fa78f5558dddca6f88ef1a
    Author: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
    Date:   Tue Jul 4 13:10:22 2023 +0600

        remote: fix stream use-after-free

When the message was processed partially, it is actually supposed to
stay in the queue to be processed again. In such case, reinsert it back.

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
---
 src/remote/remote_daemon_stream.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
index 345c40b48c..f52af790c1 100644
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -775,8 +775,12 @@ daemonStreamHandleWrite(virNetServerClient *client,
             ret = -1;
         }
 
-        if (ret > 0)
-            break;  /* still processing data from msg */
+        if (ret > 0) {
+            /* still processing data from msg, put it back into queue */
+            msg->next = stream->rx;
+            stream->rx = msg;
+            break;
+        }
 
         if (ret < 0) {
             virNetMessageFree(msg);
-- 
2.41.0
Re: [PATCH] remote: handle partial data transmission
Posted by Michal Prívozník 9 months, 2 weeks ago
On 7/26/23 09:47, Oleg Vasilev wrote:
> A new bug was introduced as a part of use-after-free fix below:
> 
>     commit 411cbe7199ce533ae5fa78f5558dddca6f88ef1a
>     Author: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
>     Date:   Tue Jul 4 13:10:22 2023 +0600
> 
>         remote: fix stream use-after-free
> 
> When the message was processed partially, it is actually supposed to
> stay in the queue to be processed again. In such case, reinsert it back.
> 
> Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
> ---
>  src/remote/remote_daemon_stream.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
> index 345c40b48c..f52af790c1 100644
> --- a/src/remote/remote_daemon_stream.c
> +++ b/src/remote/remote_daemon_stream.c
> @@ -775,8 +775,12 @@ daemonStreamHandleWrite(virNetServerClient *client,
>              ret = -1;
>          }
>  
> -        if (ret > 0)
> -            break;  /* still processing data from msg */
> +        if (ret > 0) {
> +            /* still processing data from msg, put it back into queue */
> +            msg->next = stream->rx;
> +            stream->rx = msg;
> +            break;
> +        }
>  
>          if (ret < 0) {
>              virNetMessageFree(msg);


Ah, and we have to put it back at the beginning of the queue, whereas
virNetMessageQueuePush() would put it at the end of the queue.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal