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

[Jan Kiszka]

On 2013-02-15 12:00, Vitaly Chipounov wrote:
> A socket may still have references to it in various queues
> at the time it is freed, causing memory corruptions.

Did you see it in practice? Or is this patch based on code review? What
will happen if those queued mbufs find their ifq_so NULL?

> 
> 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;
> +
> +    for (ifm = slirp->if_fastq.ifq_next; ifm != &slirp->if_fastq; ifm = 
> ifm->ifq_next) {

This line and the one below smells like checkpatch.pl wasn't run against
it. Granted, slirp patches easy make checkpatch upset as the input is
not properly formatted, but please don't add more violations.

> +        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;
> +        }
> +    }
> +}
> +
>  /*
>   * 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);
>  }
>  
> 

Sorry for the late feedback,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux