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
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
© 2016 - 2026 Red Hat, Inc.