From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 2E356A0096
	for <public@inbox.dpdk.org>; Mon,  6 May 2019 15:22:17 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 08BB53772;
	Mon,  6 May 2019 15:22:17 +0200 (CEST)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id 7F78B2082;
 Mon,  6 May 2019 15:22:13 +0200 (CEST)
Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com
 [10.5.11.23])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id C8A3081DF4;
 Mon,  6 May 2019 13:22:12 +0000 (UTC)
Received: from dhcp-25.97.bos.redhat.com (ovpn-124-136.rdu2.redhat.com
 [10.10.124.136])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E7DE1EE;
 Mon,  6 May 2019 13:22:09 +0000 (UTC)
From: Aaron Conole <aconole@redhat.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Herakliusz Lipiec <herakliusz.lipiec@intel.com>,
 dpdk stable <stable@dpdk.org>, Anatoly Burakov <anatoly.burakov@intel.com>
References: <20190506131120.25140-1-aconole@redhat.com>
 <CAJFAV8yFuKegFrmf4WHQRoCEV78RNqOZi_Oy02cMk8Rf+=z2GA@mail.gmail.com>
Date: Mon, 06 May 2019 09:22:08 -0400
In-Reply-To: <CAJFAV8yFuKegFrmf4WHQRoCEV78RNqOZi_Oy02cMk8Rf+=z2GA@mail.gmail.com>
 (David Marchand's message of "Mon, 6 May 2019 15:14:37 +0200")
Message-ID: <f7tmujziupr.fsf@dhcp-25.97.bos.redhat.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.5.110.25]); Mon, 06 May 2019 13:22:12 +0000 (UTC)
Subject: Re: [dpdk-stable] [PATCH] ipc: unlock on failure
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>

David Marchand <david.marchand@redhat.com> writes:

> On Mon, May 6, 2019 at 3:11 PM Aaron Conole <aconole@redhat.com> wrote:
>
>  Reported by Coverity.
>
>  Fixes: a2a06860b8c4 ("ipc: fix memory leak on request failure")
>  Cc: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
>  Cc: stable@dpdk.org
>  Cc: Anatoly Burakov <anatoly.burakov@intel.com>
>  Signed-off-by: Aaron Conole <aconole@redhat.com>
>  ---
>   lib/librte_eal/common/eal_common_proc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
>  diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
>  index d23728604..3498e6b07 100644
>  --- a/lib/librte_eal/common/eal_common_proc.c
>  +++ b/lib/librte_eal/common/eal_common_proc.c
>  @@ -1005,8 +1005,10 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
>                  /* unlocks the mutex while waiting for response,
>                   * locks on receive
>                   */
>  -               if (mp_request_sync(path, req, reply, &end))
>  +               if (mp_request_sync(path, req, reply, &end)) {
>  +                       pthread_mutex_unlock(&pending_requests.lock);
>                          goto err;
>  +               }
>          }
>          pthread_mutex_unlock(&pending_requests.lock);
>          /* unlock the directory */
>  -- 
>  2.19.1
>
> Unfortunately, I think this is more complex than this.
> We will leave a flock() pending and leak some resources from opendir().

I completely forgot about it, while double checking the mp_request_sync

You're right, we leak the dirfd, still.

Okay, I think see a better way to write this.