<div dir="ltr">
<div>Please ignore this patch, I'll submit an updated one. I somehow managed to only execute a subset of the fast-tests suite initially and didn't run eal_flags_misc_autotest at all. Now I see that my proposed fix is flawed, I will submit another try soon.</div><div><br></div><div>Sorry for the noise</div>
</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 3, 2023 at 11:57 AM Ashish Sadanandan <<a href="mailto:ashish.sadanandan@gmail.com">ashish.sadanandan@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The code added for allowing --huge-dir to specify hugetlbfs<br>
sub-directories has a bug where it incorrectly matches mounts that<br>
contain a prefix of the specified --huge-dir.<br>
<br>
Consider --huge-dir=/dev/hugepages1G is passed to rte_eal_init. Given<br>
the following hugetlbfs mounts<br>
<br>
$ mount | grep hugetlbfs<br>
hugetlbfs on /dev/hugepages type hugetlbfs (rw,relatime,pagesize=2M)<br>
hugetlbfs on /dev/hugepages1G type hugetlbfs (rw,relatime,pagesize=1024M)<br>
hugetlbfs on /mnt/huge type hugetlbfs (rw,relatime,pagesize=2M)<br>
<br>
get_hugepage_dir is first called with hugepage_sz=2097152. While<br>
iterating over all mount points, /dev/hugepages is incorrectly<br>
determined to be a match because it's a prefix of --huge-dir. The caller<br>
then obtains an exclusive lock on --huge-dir.<br>
<br>
In the next call to get_hugepage_dir, hugepage_sz=1073741824. This call<br>
correctly determines /dev/hugepages1G is a match. The caller again<br>
attempts to obtain an exclusive lock on --huge-dir and deadlocks because<br>
it's already holding a lock.<br>
<br>
This has been corrected by rejecting the mount point being considered if<br>
its length is smaller than the specified --huge-dir.<br>
<br>
Fixes: 24d5a1ce6b85 ("eal/linux: allow hugetlbfs sub-directories")<br>
Cc: <a href="mailto:john.levon@nutanix.com" target="_blank">john.levon@nutanix.com</a><br>
Cc: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a><br>
---<br>
lib/eal/linux/eal_hugepage_info.c | 11 +++++++----<br>
1 file changed, 7 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c<br>
index a1b6cb31ff..fcc3d82fdf 100644<br>
--- a/lib/eal/linux/eal_hugepage_info.c<br>
+++ b/lib/eal/linux/eal_hugepage_info.c<br>
@@ -269,16 +269,19 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len)<br>
* Ignore any mount that doesn't contain the --huge-dir<br>
* directory.<br>
*/<br>
- if (strncmp(internal_conf->hugepage_dir, splitstr[MOUNTPT],<br>
- strlen(splitstr[MOUNTPT])) != 0) {<br>
+ size_t mountpt_len = strlen(splitstr[MOUNTPT]);<br>
+<br>
+ if (strlen(internal_conf->hugepage_dir) > mountpt_len)<br>
+ continue;<br>
+ else if (strncmp(internal_conf->hugepage_dir, splitstr[MOUNTPT],<br>
+ mountpt_len) != 0)<br>
continue;<br>
- }<br>
<br>
/*<br>
* We found a match, but only prefer it if it's a longer match<br>
* (so /mnt/1 is preferred over /mnt for matching /mnt/1/2)).<br>
*/<br>
- if (strlen(splitstr[MOUNTPT]) > strlen(found))<br>
+ if (mountpt_len > strlen(found))<br>
strlcpy(found, splitstr[MOUNTPT], len);<br>
} /* end while fgets */<br>
<br>
-- <br>
2.27.0<br>
<br>
</blockquote></div>