2:10 a.m.
Found by repeatedly created and destroyed channels with guest agent. More
details in the patch.
Oleg Vasilev (2):
net: add debug logs
remote: fix stream use-after-free
src/remote/remote_daemon_stream.c | 13 +++++++------
src/rpc/virnetmessage.c | 5 +++++
2 files changed, 12 insertions(+), 6 deletions(-)
--
2.41.0
2:10 a.m.
New subject: [PATCH 1/2] net: add debug logs
Helped to debug next patch use-after-free.
Signed-off-by: Oleg Vasilev <oleg.vasilev(a)virtuozzo.com>
---
src/remote/remote_daemon_stream.c | 4 ++--
src/rpc/virnetmessage.c | 5 +++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
index 3b7519d2cb..38a2b6cceb 100644
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -318,8 +318,8 @@ daemonStreamFilter(virNetServerClient *client,
msg->header.serial != stream->serial)
goto cleanup;
- VIR_DEBUG("Incoming client=%p, rx=%p, serial=%u, proc=%d, status=%d",
- client, stream->rx, msg->header.proc,
+ VIR_DEBUG("Incoming client=%p, rx=%p, msg=%p, serial=%u, proc=%d,
status=%d",
+ client, stream->rx, msg, msg->header.proc,
msg->header.serial, msg->header.status);
virNetMessageQueuePush(&stream->rx, msg);
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c
index 50cc335fd6..af0f9cb30b 100644
--- a/src/rpc/virnetmessage.c
+++ b/src/rpc/virnetmessage.c
@@ -103,6 +103,8 @@ void virNetMessageQueuePush(virNetMessage **queue, virNetMessage
*msg)
{
virNetMessage *tmp = *queue;
+ VIR_DEBUG("queue=%p msg=%p", queue, msg);
+
if (tmp) {
while (tmp->next)
tmp = tmp->next;
@@ -117,10 +119,13 @@ virNetMessage *virNetMessageQueueServe(virNetMessage **queue)
{
virNetMessage *tmp = *queue;
+ VIR_DEBUG("queue serve start queue=%p *queue=%p", queue, *queue);
+
if (tmp) {
*queue = g_steal_pointer(&tmp->next);
}
+ VIR_DEBUG("queue serve end queue=%p *queue=%p", queue, *queue);
return tmp;
}
--
2.41.0
2:10 a.m.
New subject: [PATCH 2/2] remote: fix stream use-after-free
Inside daemonStreamHandleWrite on stream completion (status=OK) we
reuse msg object to send confirmation.
Only after that, msg is poped from the queue and checked for continue.
By that time, msg might've already been processed for the confirmation
and freed.
Signed-off-by: Oleg Vasilev <oleg.vasilev(a)virtuozzo.com>
---
src/remote/remote_daemon_stream.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
index 38a2b6cceb..345c40b48c 100644
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -740,10 +740,11 @@ static int
daemonStreamHandleWrite(virNetServerClient *client,
daemonClientStream *stream)
{
+ virNetMessageStatus status = VIR_NET_OK;
VIR_DEBUG("client=%p, stream=%p", client, stream);
while (stream->rx && !stream->closed) {
- virNetMessage *msg = stream->rx;
+ virNetMessage *msg = virNetMessageQueueServe(&stream->rx);
int ret;
if (msg->header.type == VIR_NET_STREAM_HOLE) {
@@ -752,7 +753,8 @@ daemonStreamHandleWrite(virNetServerClient *client,
* data. */
ret = daemonStreamHandleHole(client, stream, msg);
} else if (msg->header.type == VIR_NET_STREAM) {
- switch (msg->header.status) {
+ status = msg->header.status;
+ switch (status) {
case VIR_NET_OK:
ret = daemonStreamHandleFinish(client, stream, msg);
break;
@@ -776,7 +778,6 @@ daemonStreamHandleWrite(virNetServerClient *client,
if (ret > 0)
break; /* still processing data from msg */
- virNetMessageQueueServe(&stream->rx);
if (ret < 0) {
virNetMessageFree(msg);
virNetServerClientImmediateClose(client);
@@ -789,7 +790,7 @@ daemonStreamHandleWrite(virNetServerClient *client,
* onto the wire, but this causes the client to reset
* its active request count / throttling
*/
- if (msg->header.status == VIR_NET_CONTINUE) {
+ if (status == VIR_NET_CONTINUE) {
virNetMessageClear(msg);
msg->header.type = VIR_NET_REPLY;
if (virNetServerClientSendMessage(client, msg) < 0) {
--
2.41.0
1:49 a.m.
New subject: PING [PATCH 0/2] fix use-after-free in network IO
On 04/07/2023 13:10, Oleg Vasilev wrote:
Found by repeatedly created and destroyed channels with guest agent.
More
details in the patch.
Oleg Vasilev (2):
net: add debug logs
remote: fix stream use-after-free
src/remote/remote_daemon_stream.c | 13 +++++++------
src/rpc/virnetmessage.c | 5 +++++
2 files changed, 12 insertions(+), 6 deletions(-)
4:50 a.m.
New subject: PING 2 [PATCH 0/2] fix use-after-free in network IO
Please look at.
On 13/07/2023 12:49, Oleg Vasilev wrote:
On 04/07/2023 13:10, Oleg Vasilev wrote:
> Found by repeatedly created and destroyed channels with guest agent. More
> details in the patch.
>
> Oleg Vasilev (2):
> net: add debug logs
> remote: fix stream use-after-free
>
> src/remote/remote_daemon_stream.c | 13 +++++++------
> src/rpc/virnetmessage.c | 5 +++++
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
7:32 a.m.
On 7/4/23 09:10, Oleg Vasilev wrote:
Found by repeatedly created and destroyed channels with guest agent.
More
details in the patch.
Oleg Vasilev (2):
net: add debug logs
remote: fix stream use-after-free
src/remote/remote_daemon_stream.c | 13 +++++++------
src/rpc/virnetmessage.c | 5 +++++
2 files changed, 12 insertions(+), 6 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal
488
days inactive
508
days old
5 comments
2 participants
participants (2)
-
Michal Prívozník -
Oleg Vasilev