From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 074FE532C for ; Wed, 26 Jul 2017 11:06:22 +0200 (CEST) Received: by mail-wm0-f52.google.com with SMTP id m85so61652619wma.1 for ; Wed, 26 Jul 2017 02:06:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=HYEoYoQ4/WcMszPSXJ4sQvhUFhITsDVX8e/8VHZLiCQ=; b=t762m7Mx0tQ0YmVyXo4/p3Aa21yA0/Nn6Tz1+J9ze508fnGqqYUSgLXXg8BdFm6j0M qgP0hMMcWvsf+9EYklibHLecnbyBDupGAMSVzbvTuGQL9cOM5FjM6NqOgIGwWLCFNPA6 Cq4jBVdFA6aUsvc3HSboBZNUN4FBurPnUrVeCdg5NiRTvsKgwDZWobJf2j3hqPw/u/lb EvbBlqRDJ3djWGpdo6p6JaYESoahVwfYHIqK6ZjIgc7LiNsMJ4HE3rJsfev3fBLG6AWB X9Rlwo5BEeVW+6QrVx48ufeTxDmLulA010RM8mcwkpSe6dUsqQEh6MyzjXJXrsL8F0A1 GsKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=HYEoYoQ4/WcMszPSXJ4sQvhUFhITsDVX8e/8VHZLiCQ=; b=MXbyBE2lvTnc90NuvJ5NaHLDvhmR3K6hdtLTrgrN1CHyAoG1k+h3VbJ7CmgC71Gu8M Jqm3YzOolOu8UEXYcyxm9EHgz73QsUNdLMAR5Z4QBVE0HHL0K8SDcfXdZCCdttdnLvem jFc0Z6ZgI/cutg853o89uTXfJV4Bf7yySPAtwElK3wbxJ4TNKZehBsiDvaIQ+jKl86j4 kbF7kvsqq6LZ86lwacCq+XQ0KGHz+y1mvLTHsnjz0LIY2hY0NyPXhUfSuSiaSYX5ueQ7 vfKInqXs2iZXaaKHTOMp+3mmwjR9gtW+HSMvlCigaa2N2taHxEZ6ab9iRDEH/LXlCU5D U+aA== X-Gm-Message-State: AIVw112TqGSY6y2VKKDQ7vNA0nVImSpw3H+omqJltoXGB2oXykWQIQou UlgGgXl8yt4x68TVEVg= X-Received: by 10.28.142.138 with SMTP id q132mr178134wmd.75.1501059982592; Wed, 26 Jul 2017 02:06:22 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id y12sm32867292wrb.39.2017.07.26.02.06.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 26 Jul 2017 02:06:21 -0700 (PDT) Date: Wed, 26 Jul 2017 11:06:13 +0200 From: Adrien Mazarguil To: Shachar Beiser Cc: dev@dpdk.org, Nelio Laranjeiro , stable@dpdk.org Message-ID: <20170726090613.GT19852@6wind.com> References: <5ec0604196fb087a42fa75e6ddc2a04aab293593.1501047767.git.shacharbe@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5ec0604196fb087a42fa75e6ddc2a04aab293593.1501047767.git.shacharbe@mellanox.com> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix verification of device context X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 09:06:23 -0000 Hi Shachar, On Wed, Jul 26, 2017 at 05:43:24AM +0000, Shachar Beiser wrote: > Get interface name function lacks verification of device context. > It might lead to segmentation fault when trying to query the name > after the device is closed.fixing it by adding the missing verification > Thanks, however if by "close" you mean it may occur when applications use ethdev callbacks after a call to rte_eth_dev_close(), I do not think PMDs have to protect themselves against bad application behavior, otherwise there is no end to such fixes. The reverse of rte_eth_dev_close() is not rte_eth_dev_configure() nor any other ethdev callback (see documentation), but a bus probe operation. Perhaps I've missed something, so in case a crash occurs *while* calling rte_eth_dev_close() I guess this patch is fine, but then please describe the reason. > Fixes: cd89f22a1e9770 ("net/mlx5: remove unused interface name query") This commit doesn't look like the root cause of that issue? > Cc: stable@dpdk.org > > Signed-off-by: Shachar Beiser > --- > drivers/net/mlx5/mlx5_ethdev.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > index b70b7b9..6e67461 100644 > --- a/drivers/net/mlx5/mlx5_ethdev.c > +++ b/drivers/net/mlx5/mlx5_ethdev.c > @@ -173,6 +173,10 @@ struct priv * > char match[IF_NAMESIZE] = ""; > > { > + if (priv->ctx == NULL) { > + DEBUG("The device is closed, cannot query interface name "); > + return -1; > + } MKSTR() is at the beginning of this block because it defines a new variable (path). For coding style consistency you should not put any code before variable declarations, or at least insert an empty line between them. Otherwise you could move this check to the parent block. > MKSTR(path, "%s/device/net", priv->ctx->device->ibdev_path); > > dir = opendir(path); > -- > 1.8.3.1 > I think this patch is not necessary unless proved otherwise, have you actually observed a crash addressed by it? -- Adrien Mazarguil 6WIND