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 0AAFBA00C2; Wed, 4 Jan 2023 12:25:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9CC7840A82; Wed, 4 Jan 2023 12:24:59 +0100 (CET) Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by mails.dpdk.org (Postfix) with ESMTP id D5F0340697; Wed, 4 Jan 2023 12:24:58 +0100 (CET) Received: by mail-lf1-f46.google.com with SMTP id bt23so33314156lfb.5; Wed, 04 Jan 2023 03:24:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=e+kumV9ke0iMuxXb6M0ghMAnyPFwCgxMESxsOjTE8co=; b=hoM0biqc6MopQSZwlYp5kWQxL/q2oxO4Op6yuxkRTS6Ngj0ctItWDM3Hg7/3iBqsZy 1PoazOxdEjgI65INjptt6iVpz22yuzgwtiSv77DLTy3FKmLhQFyj+x0ywfjs4F2IxqlS xqUt5Wx45xWayeuSt4gPyYf3Tavp4SU/tCKARtKX1FuuY6x3oFbecl9f+uw/viakLEGI +nidafZm/IF5b/19PKFTtAl6c1xDKqc8vg3+Q1XaSEIVm1hCiLx4++adICQ/Yf2Pxs70 NTfyh0E6kwJgMykjnauZdbSj/WGEnXKlhJ1M38HoBMP54BFmMjM1kkb2YAL2VC9H/s2A EsoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=e+kumV9ke0iMuxXb6M0ghMAnyPFwCgxMESxsOjTE8co=; b=ihMgbNb4RGL1+tw2+PxoSh2B837x2LWjtGiw5AKKcz7IeReBv1j1Uhc4FTmH998G82 vUFi9LqYaFwCasmkkhOgd/gKB+rNSlYzGdRJIBHS/4KedN7KhDrmXts6R7p6+AwF9J2J CmrGcGmZACZz7LlmewBTGGC16xAcm57SfGMdMwEsSPKt9GN2r/66HWUMtt1kRYyg1Evf +Z/Il1QadOxm7Ql350DQLRxxmsS7hIp7RToUWmE8UFrO6ARpYVI1HeEdCfJOCjV0W9/1 jr8cO5JgzsIKG0cmMBaPj+UNlS9zlReqL+KD84ft91UkPPtJXPChV1y0H4aYAz9+H28Z PaAw== X-Gm-Message-State: AFqh2kqXyZek855yk80n6lr++zqyVlUj7+4jFmQaIp2vKX8zxdeAdt5g YXx8mjlMXsPd3eNjR9OxaJk= X-Google-Smtp-Source: AMrXdXtRnVtAxnkTx7yH2FsG9S72mv2f0hjS9U5ZqAB05hx0Sb/DvT6U3jx8ktgRzHYZDsBAol2+zw== X-Received: by 2002:a05:6512:3e0e:b0:4a4:68b9:1a14 with SMTP id i14-20020a0565123e0e00b004a468b91a14mr15841438lfv.60.1672831498182; Wed, 04 Jan 2023 03:24:58 -0800 (PST) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id x2-20020a0565123f8200b004b590b0c084sm5038022lfa.3.2023.01.04.03.24.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Jan 2023 03:24:57 -0800 (PST) Date: Wed, 4 Jan 2023 14:24:56 +0300 From: Dmitry Kozlyuk To: Ashish Sadanandan Cc: dev@dpdk.org, john.levon@nutanix.com, stable@dpdk.org Subject: Re: [PATCH v2 1/1] eal/linux: reject --huge-dir not parent of mountpt Message-ID: <20230104142456.320fc95a@sovereign> In-Reply-To: <20230104000030.187857-1-ashish.sadanandan@gmail.com> References: <20230103185732.2007210-1-ashish.sadanandan@gmail.com> <20230104000030.187857-1-ashish.sadanandan@gmail.com> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 2023-01-03 17:00 (UTC-0700), Ashish Sadanandan: > The code added for allowing --huge-dir to specify hugetlbfs > sub-directories has a bug where it incorrectly matches mounts that > contain a prefix of the specified --huge-dir. > > Consider --huge-dir=/dev/hugepages1G is passed to rte_eal_init. Given > the following hugetlbfs mounts > > $ mount | grep hugetlbfs > hugetlbfs on /dev/hugepages type hugetlbfs (rw,relatime,pagesize=2M) > hugetlbfs on /dev/hugepages1G type hugetlbfs (rw,relatime,pagesize=1024M) > hugetlbfs on /mnt/huge type hugetlbfs (rw,relatime,pagesize=2M) > > get_hugepage_dir is first called with hugepage_sz=2097152. While > iterating over all mount points, /dev/hugepages is incorrectly > determined to be a match because it's a prefix of --huge-dir. The caller > then obtains an exclusive lock on --huge-dir. > > In the next call to get_hugepage_dir, hugepage_sz=1073741824. This call > correctly determines /dev/hugepages1G is a match. The caller again > attempts to obtain an exclusive lock on --huge-dir and deadlocks because > it's already holding a lock. > > This has been corrected by ensuring any matched mount point is either an > exact match or a parent path of --huge-dir. > > Fixes: 24d5a1ce6b85 ("eal/linux: allow hugetlbfs sub-directories") > Cc: john.levon@nutanix.com > Cc: stable@dpdk.org > Signed-off-by: Ashish Sadanandan Acked-by: Dmitry Kozlyuk > --- > lib/eal/linux/eal_hugepage_info.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c > index a1b6cb31ff..180abd930c 100644 > --- a/lib/eal/linux/eal_hugepage_info.c > +++ b/lib/eal/linux/eal_hugepage_info.c > @@ -265,12 +265,23 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len) > break; > } > > + size_t mountpt_len = strlen(splitstr[MOUNTPT]); > + size_t hugepage_dir_len = strlen(internal_conf->hugepage_dir); The second one can be done before the loop. Please declare all variables at the beginning of the block per code style. > + > /* > * Ignore any mount that doesn't contain the --huge-dir > * directory. > */ > if (strncmp(internal_conf->hugepage_dir, splitstr[MOUNTPT], > - strlen(splitstr[MOUNTPT])) != 0) { > + mountpt_len) != 0) { > + continue; > + } > + /* > + * Ignore any mount where hugepage_dir is not a parent path of > + * the mount > + */ > + else if(hugepage_dir_len > mountpt_len && > + internal_conf->hugepage_dir[mountpt_len] != '/') { Nit: maybe a single comment for both conditions would be more clear: /* * Ignore any mount that is not --huge-dir or its subdirectory. */ > continue; > } > > @@ -278,7 +289,7 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len) > * We found a match, but only prefer it if it's a longer match > * (so /mnt/1 is preferred over /mnt for matching /mnt/1/2)). > */ > - if (strlen(splitstr[MOUNTPT]) > strlen(found)) > + if (mountpt_len > strlen(found)) > strlcpy(found, splitstr[MOUNTPT], len); > } /* end while fgets */