From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56904) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anFFQ-0006iM-5d for qemu-devel@nongnu.org; Mon, 04 Apr 2016 20:53:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anFFM-0001MU-5s for qemu-devel@nongnu.org; Mon, 04 Apr 2016 20:53:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55501) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anFFM-0001Lh-01 for qemu-devel@nongnu.org; Mon, 04 Apr 2016 20:53:32 -0400 References: <1459494489-3532-1-git-send-email-zhang.zhanghailiang@huawei.com> <56FE25B2.7000203@redhat.com> <56FE3025.4010902@huawei.com> <56FE3AE8.6060109@cn.fujitsu.com> <56FE4077.40507@huawei.com> From: Jason Wang Message-ID: <57030C82.5080108@redhat.com> Date: Tue, 5 Apr 2016 08:53:22 +0800 MIME-Version: 1.0 In-Reply-To: <56FE4077.40507@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] filter-buffer: fix segfault while start qemu with status=off property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hailiang Zhang , Wen Congyang , qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com On 04/01/2016 05:33 PM, Hailiang Zhang wrote: > On 2016/4/1 17:10, Wen Congyang wrote: >> On 04/01/2016 04:24 PM, Hailiang Zhang wrote: >>> On 2016/4/1 15:39, Jason Wang wrote: >>>> >>>> >>>> On 04/01/2016 03:08 PM, zhanghailiang wrote: >>>>> After commit 338d3f, we support 'status' property for filter object. >>>>> The segfault can be triggered by starting qemu with 'status=off' >>>>> property >>>>> for filter, when the s->incoming_queue is NULL, we reference it >>>>> directly >>>>> in qemu_net_queue_flush(). >>>>> >>>>> Let's check the value of 's->incoming_queue' before calling >>>>> qemu_net_queue_flush(). >>>>> >>>>> Signed-off-by: zhanghailiang >>>>> --- >>>>> net/filter-buffer.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >>>>> index cc6bd94..79e2ce3 100644 >>>>> --- a/net/filter-buffer.c >>>>> +++ b/net/filter-buffer.c >>>>> @@ -34,7 +34,7 @@ static void filter_buffer_flush(NetFilterState *nf) >>>>> { >>>>> FilterBufferState *s = FILTER_BUFFER(nf); >>>>> >>>>> - if (!qemu_net_queue_flush(s->incoming_queue)) { >>>>> + if (s->incoming_queue && >>>>> !qemu_net_queue_flush(s->incoming_queue)) { >>>>> /* Unable to empty the queue, purge remaining packets */ >>>>> qemu_net_queue_purge(s->incoming_queue, nf->netdev); >>>>> } >>>> >>>> We'd better handle this at generic layer and don't let a specific net >>>> filter need to worry about this. >>>> >>>> Looks like the issue is we may trigger status_changed() too early >>>> (even >>>> before the the filter was initialized). >>>> >>> >>> Yes ~ >>> >>>> How about not call status_changed() if the initialization is not done? >>>> >>> >>> But seems that it is difficult to confirm if the filter is initialized >>> or not ... >> >> If nfc->setup() is not called, nf->netdev is NULL. >> > > Yes, you right, Jason, what's opinion ? > > Thanks, > hailiang Looks good. Please also add a comment of this in the patch. Thanks > >> Thanks >> Wen Congyang >> >>> >>>> . >>>> >>> >>> >>> >>> >>> >> >> >> >> >> . >> > >