| Message ID | 20170824234241.5cfnuic3lmcea6xc@var.youpi.perso.aquilenet.fr |
|---|---|
| State | New |
| Headers | show |
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 <samuel.thibault@ens-lyon.org> > 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 <samuel.thibault@ens-lyon.org> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > 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; > + } > } > } > >
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; + } } }
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? Samuel commit 1a3a763509fad895c907e6978ea034a5c19ee370 Author: Samuel Thibault <samuel.thibault@ens-lyon.org> 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 <samuel.thibault@ens-lyon.org>