From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZBsP-00010A-QQ for qemu-devel@nongnu.org; Fri, 26 Feb 2016 01:27:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aZBsM-0005QK-8b for qemu-devel@nongnu.org; Fri, 26 Feb 2016 01:27:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54294) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZBsM-0005QG-3A for qemu-devel@nongnu.org; Fri, 26 Feb 2016 01:27:42 -0500 References: <1456285491-6952-1-git-send-email-jasowang@redhat.com> From: Jason Wang Message-ID: <56CFF054.6080606@redhat.com> Date: Fri, 26 Feb 2016 14:27:32 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] net: filter: correctly remove filter from the list during finalization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang Cc: qemu-devel@nongnu.org On 02/24/2016 07:53 PM, Yang Hongyang wrote: > > On Wed, Feb 24, 2016 at 11:44 AM, Jason Wang > wrote: > > Qemu may crash when we want to add two filters on the same netdev b= ut > the initialization of second fails (e.g missing parameters): > > ./qemu-system-x86_64 -netdev user,id=3Dun0 \ > -object filter-buffer,id=3Df0,netdev=3Dun0,interval=3D10 \ > -object filter-buffer,id=3Df1,netdev=3Dun0 > Segmentation fault (core dumped) > > This is because we don't check whether or not the filter was in the > list of netdev. This patch fixes this. > > > Oops, thanks for catching this! > =20 > > > Cc: Yang Hongyang > > Signed-off-by: Jason Wang >=20 > > --- > net/filter.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/filter.c b/net/filter.c > index d2a514e..7cdbc6c 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -196,7 +196,8 @@ static void netfilter_finalize(Object *obj) > nfc->cleanup(nf); > } > > - if (nf->netdev && !QTAILQ_EMPTY(&nf->netdev->filters)) { > + if (nf->netdev && !QTAILQ_EMPTY(&nf->netdev->filters) && > + nf->next.tqe_prev) { > > > Using queue's inner member tqe_prev directly might not be a good idea=EF= =BC=8Cbut > seems there's no better way to do this. > Are there any chance that we could add a QTAILQ_XXX helper to check > whether a > member is in the queue or not? Might be a good idea, but I'm not sure. > Just some thoughts, I'm ok with the current patch though, so: > > Reviewed-by: Yang Hongyang > Applied to -net. Thanks > =20 > > QTAILQ_REMOVE(&nf->netdev->filters, nf, next); > } > g_free(nf->netdev_id); > -- > 2.5.0 > > > > > > --=20 > Thanks, > Yang