qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] slirp: fixed potential use-after-free of a sock


From: mdroth
Subject: Re: [Qemu-devel] [PATCH] slirp: fixed potential use-after-free of a socket
Date: Thu, 21 Feb 2013 15:47:25 -0600
User-agent: Mutt/1.5.21 (2010-09-15)

On Fri, Feb 15, 2013 at 12:00:13PM +0100, Vitaly Chipounov wrote:
> A socket may still have references to it in various queues
> at the time it is freed, causing memory corruptions.
> 
> Signed-off-by: Vitaly Chipounov <address@hidden>
> ---
>  slirp/socket.c |   29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 77b0c98..8a7adc8 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -55,6 +55,33 @@ socreate(Slirp *slirp)
>    return(so);
>  }
> 
> +/**
> + * It may happen that a socket is still referenced in various
> + * mbufs at the time it is freed. Clear all references to the
> + * socket here.
> + */
> +static void soremovefromqueues(struct socket *so)
> +{
> +    if (!so->so_queued) {
> +        return;
> +    }
> +
> +    Slirp *slirp = so->slirp;
> +    struct mbuf *ifm;

Declarations should come at the beginning of a block/function (see: ...
er, I could've sworn it was in HACKING or CODING_STYLE but maybe not.
That's the standard in any case)

> +
> +    for (ifm = slirp->if_fastq.ifq_next; ifm != &slirp->if_fastq; ifm = 
> ifm->ifq_next) {
> +        if (ifm->ifq_so == so) {
> +            ifm->ifq_so = NULL;
> +        }
> +    }
> +
> +    for (ifm = slirp->if_batchq.ifq_next; ifm != &slirp->if_batchq; ifm = 
> ifm->ifq_next) {
> +        if (ifm->ifq_so == so) {
> +            ifm->ifq_so = NULL;
> +        }
> +    }

Is the intention just to mark them null or actually remove? Not sure
which, but either the logic should change or the function name should
(soinvalidate() or something along that line if the former).

> +}
> +
>  /*
>   * remque and free a socket, clobber cache
>   */
> @@ -79,6 +106,8 @@ sofree(struct socket *so)
>    if(so->so_next && so->so_prev)
>      remque(so);  /* crashes if so is not in a queue */
> 
> +  soremovefromqueues(so);
> +
>    free(so);
>  }
> 
> -- 
> 1.7.10
> 
> 



reply via email to

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