On Thu, Feb 21, 2013 at 03:47:25PM -0600, mdroth wrote:
> 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 <vitaly.chipou...@epfl.ch>
Meant to cc qemu-stable
> > ---
> > 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
> >
> >
