[Qemu-devel] [PATCH] slirp: Fix access to freed memory

[Qemu-devel] [PATCH] slirp: Fix access to freed memory

[Samuel Thibault]

if_start() goes through the slirp->if_fastq and slirp->if_batchq
list of pending messages, and accesses ifm->ifq_so->so_nqueued of its
elements if ifm->ifq_so != NULL.  When freeing a socket, we thus need
to make sure that any pending message for this socket does not refer
to the socket any more.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Brian Candler <b.candler@pobox.com>
---
 slirp/socket.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/slirp/socket.c b/slirp/socket.c
index 280050a..6c18971 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -66,6 +66,23 @@ 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;
+    }
+  }
+
+  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;
+    }
+  }
 
   if (so->so_emu==EMU_RSH && so->extra) {
 	sofree(so->extra);
-- 
2.10.2

Re: [Qemu-devel] [PATCH] slirp: Fix access to freed memory

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] slirp: Fix access to freed memory
Message-id: 20161113230102.12173-1-samuel.thibault@ens-lyon.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20161113230102.12173-1-samuel.thibault@ens-lyon.org -> patchew/20161113230102.12173-1-samuel.thibault@ens-lyon.org
Switched to a new branch 'test'
8fdbb81 slirp: Fix access to freed memory

=== OUTPUT BEGIN ===
fatal: unrecognized argument: --no-patch
Checking PATCH 1/1: ...
ERROR: suspect code indent for conditional statements (4, 6)
#29: FILE: slirp/socket.c:74:
+    if (ifm->ifq_so == so) {
+      ifm->ifq_so = NULL;

ERROR: suspect code indent for conditional statements (4, 6)
#37: FILE: slirp/socket.c:82:
+    if (ifm->ifq_so == so) {
+      ifm->ifq_so = NULL;

total: 2 errors, 0 warnings, 23 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH] slirp: Fix access to freed memory

[Samuel Thibault]

Hello,

Note:

no-reply@patchew.org, on Sun 13 Nov 2016 15:13:47 -0800, wrote:
> Your series seems to have some coding style problems. See output below for
> more information:
> 
> === OUTPUT BEGIN ===
> fatal: unrecognized argument: --no-patch
> Checking PATCH 1/1: ...
> ERROR: suspect code indent for conditional statements (4, 6)
> #29: FILE: slirp/socket.c:74:
> +    if (ifm->ifq_so == so) {
> +      ifm->ifq_so = NULL;
> 
> ERROR: suspect code indent for conditional statements (4, 6)
> #37: FILE: slirp/socket.c:82:
> +    if (ifm->ifq_so == so) {
> +      ifm->ifq_so = NULL;

This is due to that portion of the slirp code using 2-space indentation
instead of 4-space indentation.

Samuel

Re: [Qemu-devel] [PATCH] slirp: Fix access to freed memory

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

On Mon, Nov 14, 2016 at 12:01:02AM +0100, Samuel Thibault wrote:
> if_start() goes through the slirp->if_fastq and slirp->if_batchq
> list of pending messages, and accesses ifm->ifq_so->so_nqueued of its
> elements if ifm->ifq_so != NULL.  When freeing a socket, we thus need
> to make sure that any pending message for this socket does not refer
> to the socket any more.
> 
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Tested-by: Brian Candler <b.candler@pobox.com>
> ---
>  slirp/socket.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Please send a slirp pull request for QEMU 2.8-rc0 (deadline November
15th) or -rc1 (deadline November 22nd).

CCing qemu-stable.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]