Re: [Qemu-devel] [PATCH] net: filter: correctly remove filter from the list during finalization

[Jason Wang <jasowang@redhat.com>]

On 02/24/2016 07:53 PM, Yang Hongyang wrote:
>
> On Wed, Feb 24, 2016 at 11:44 AM, Jason Wang <jasowang@redhat.com
> <mailto:jasowang@redhat.com>> wrote:
>
>     Qemu may crash when we want to add two filters on the same netdev but
>     the initialization of second fails (e.g missing parameters):
>
>     ./qemu-system-x86_64 -netdev user,id=un0 \
>      -object filter-buffer,id=f0,netdev=un0,interval=10 \
>      -object filter-buffer,id=f1,netdev=un0
>     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!
>  
>
>
>     Cc: Yang Hongyang <hongyang.yang@easystack.cn
>     <mailto:hongyang.yang@easystack.cn>>
>     Signed-off-by: Jason Wang <jasowang@redhat.com
>     <mailto:jasowang@redhat.com>> 
>
>     ---
>      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，but
> 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 <hongyang.yang@easystack.cn
> <mailto:hongyang.yang@easystack.cn>>

Applied to -net.

Thanks

>  
>
>              QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
>          }
>          g_free(nf->netdev_id);
>     --
>     2.5.0
>
>
>
>
>
> -- 
> Thanks,
> Yang