From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 88BF4A0093; Tue, 3 May 2022 16:43:04 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3264340C35; Tue, 3 May 2022 16:43:04 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id E50C440691 for ; Tue, 3 May 2022 16:43:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1651588981; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VGVcBkFBAfjHLM1uXQGK6TXuiUOvNlKZTfAx4tY+Lus=; b=VrnBd0q3Z6IiUrPOZ3LqvRRWwM2O2HGoUcSoHpKUzY/vBfuz6xcGpuQD60yXKmvMgCUzjJ BWbp3P+Rkj1dwoOTHRLFmYjzYZsfGrI7ctDThjNPV8C9CPt9n7SRwKsqH3RwLb8cgv6Kaj ScU6vn37Vg8RHxRurax7vfHH76CStmM= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-467-RkryevCzMSWaIQEA42FreA-1; Tue, 03 May 2022 10:43:00 -0400 X-MC-Unique: RkryevCzMSWaIQEA42FreA-1 Received: by mail-lf1-f72.google.com with SMTP id d4-20020a05651221c400b0046ba0689478so8182923lft.21 for ; Tue, 03 May 2022 07:42:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VGVcBkFBAfjHLM1uXQGK6TXuiUOvNlKZTfAx4tY+Lus=; b=PT7WB/U8DYn6XCzs+CkM+32NzBvFYlpEEh4D8AdtjI+xnL+oII3DPNPUONjfsEHuK5 mnJLerKtCAFkhrisUvbRMQ9zyg5r1TAINuHM9B3npsPmou9xpgBkfJzpe0HWwb8h/fyQ cWH985EbuBOf+eM8kV7iryKV5Qj7e/mHrDK7+asLEJD3aQqEmYG40OGtEjp7dS/GkRrI mBoJVy4LFAU6fjtb5ZR6ZVLi4FKOdGMFryrDyp7kwBkVGbiV7yFYEY4qrjNpMkK06TQ+ 3swRaRArCgsWU3qbaRAuj5DsdiZYsexAuduLtiFPIJJsXtRA3dW45rWHm5brHUQCEUPf eDaw== X-Gm-Message-State: AOAM530KABSaR4UzRO5HJRAIbjjx7x9Uat+i+YcCNF2ugqkDbrp3S70M WjpROPDyj+MNdDXcjNDKXJap9x81z6wLS3JfltmI4R7/bNAdhgcLY1Asq1Jviu2fsz4tQWDHXs6 t2X2aW+6RwKlEBF11YU8= X-Received: by 2002:ac2:4bc1:0:b0:472:588e:8cae with SMTP id o1-20020ac24bc1000000b00472588e8caemr10425751lfq.575.1651588216852; Tue, 03 May 2022 07:30:16 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwKSoZbwZmw1dRpvxPFbjG9eM81oPajbssam7wkOVn/GnCh3SSnAuRz1mSrfsDddGuz0Zm+dCffAAimuwFgSj8= X-Received: by 2002:ac2:4bc1:0:b0:472:588e:8cae with SMTP id o1-20020ac24bc1000000b00472588e8caemr10425740lfq.575.1651588216607; Tue, 03 May 2022 07:30:16 -0700 (PDT) MIME-Version: 1.0 References: <20220502192238.348172-1-quentin@armitage.org.uk> <20220502192238.348172-2-quentin@armitage.org.uk> In-Reply-To: <20220502192238.348172-2-quentin@armitage.org.uk> From: David Marchand Date: Tue, 3 May 2022 16:30:05 +0200 Message-ID: Subject: Re: [PATCH 1/1] tap: fix write-after-free and double free of intr_handle To: Quentin Armitage Cc: dev , Harman Kalra Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, May 2, 2022 at 9:23 PM Quentin Armitage wrote: > > rte_pmd_tun/tap_probe() allocates pmd->intr_handle in > eth_dev_tap_create() and it should not be freed until > rte_pmd_tap_remove() is called. > > Inspection of tap_rx_intr_vec_set() shows that the call to > tap_tx_intr_vec_uninstall() was calling rte_intr_instance_free() but > tap_tx_intr_vec_install() can then be immediately called, and this then > uses pmd->intr_handle without it being reallocated. > > This commit moves the call of rte_intr_instance_free() from > tap_tx_intr_vec_uninstall() to rte_pmd_tap_remove(). Good catch. We want this fix backported: Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle") Cc: stable@dpdk.org > > Signed-off-by: Quentin Armitage > --- > drivers/net/tap/rte_eth_tap.c | 5 +++++ > drivers/net/tap/tap_intr.c | 2 -- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > index bc3d56a311..aab1692ebf 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -2612,14 +2612,19 @@ static int > rte_pmd_tap_remove(struct rte_vdev_device *dev) > { > struct rte_eth_dev *eth_dev = NULL; > + struct pmd_internals *pmd; > + struct rte_intr_handle *intr_handle; > > /* find the ethdev entry */ > eth_dev = rte_eth_dev_allocated(rte_vdev_device_name(dev)); > if (!eth_dev) > return 0; > > + pmd = eth_dev->data->dev_private; > + intr_handle = pmd->intr_handle; > tap_dev_close(eth_dev); > rte_eth_dev_release_port(eth_dev); > + rte_intr_instance_free(intr_handle); intr_handle is shared for multiprocess, so freeing should happen in the primary process only. For this reason, rte_intr_instance_free() should probably go to tap_dev_close. > > return 0; > } > diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c > index 56c343acea..a9097def1a 100644 > --- a/drivers/net/tap/tap_intr.c > +++ b/drivers/net/tap/tap_intr.c > @@ -34,8 +34,6 @@ tap_rx_intr_vec_uninstall(struct rte_eth_dev *dev) > rte_intr_free_epoll_fd(intr_handle); > rte_intr_vec_list_free(intr_handle); > rte_intr_nb_efd_set(intr_handle, 0); > - > - rte_intr_instance_free(intr_handle); > } > > /** > -- > 2.34.1 > -- David Marchand