qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] A use-after-free in slirp


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] A use-after-free in slirp
Date: Thu, 24 Aug 2017 23:51:32 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

Hi Samuel,

On 08/24/2017 08:42 PM, Samuel Thibault wrote:
Hello,

Thanks for the reproducer you sent me offline. Here is a fix which makes
a lot of sense and fixes the reproducer.  Could you try it with your
whole testcase?

Could somebody also review the patch?


Your patch looks correct.

It seems worth to add a soqfree() function for readability:

static inline void soqfree(struct quehead *ifq)
{
    struct mbuf *ifm;

    for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
        ifm->ifq_so = NULL;
    }
    ifq->ifq_so = NULL;
}

then use:

    if (ifq->ifq_so == so) {
        soqfree(ifq);
    }

same with slirp->if_batchq

Also can you send your patch in the proper format?

Thanks!

Regards,

Phil.

Samuel

commit 1a3a763509fad895c907e6978ea034a5c19ee370
Author: Samuel Thibault <address@hidden>
Date:   Fri Aug 25 01:35:53 2017 +0200

     slirp: fix clearing ifq_so from pending packets
The if_fastq and if_batchq contain not only packets, but queues of packets
     for the same socket. When sofree frees a socket, it thus has to clear 
ifq_so
     from all the packets from the queues, not only the first.
Signed-off-by: Samuel Thibault <address@hidden>

Acked-by: Philippe Mathieu-Daudé <address@hidden>


diff --git a/slirp/socket.c b/slirp/socket.c
index ecec0295a9..4203b80542 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -66,21 +66,29 @@ void
  sofree(struct socket *so)
  {
    Slirp *slirp = so->slirp;
-  struct mbuf *ifm;
-
-  for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
-       (struct quehead *) ifm != &slirp->if_fastq;
-       ifm = ifm->ifq_next) {
-    if (ifm->ifq_so == so) {
-      ifm->ifq_so = NULL;
+  struct mbuf *ifq;
+
+  for (ifq = (struct mbuf *) slirp->if_fastq.qh_link;
+       (struct quehead *) ifq != &slirp->if_fastq;
+       ifq = ifq->ifq_next) {
+    if (ifq->ifq_so == so) {
+      struct mbuf *ifm;
+      ifq->ifq_so = NULL;
+      for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+        ifm->ifq_so = NULL;
+      }
      }
    }
- for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
-       (struct quehead *) ifm != &slirp->if_batchq;
-       ifm = ifm->ifq_next) {
-    if (ifm->ifq_so == so) {
-      ifm->ifq_so = NULL;
+  for (ifq = (struct mbuf *) slirp->if_batchq.qh_link;
+       (struct quehead *) ifq != &slirp->if_batchq;
+       ifq = ifq->ifq_next) {
+    if (ifq->ifq_so == so) {
+      struct mbuf *ifm;
+      ifq->ifq_so = NULL;
+      for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+        ifm->ifq_so = NULL;
+      }
      }
    }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]