[v2,04/10] eal: fix plugin dir walk

For '.' and '..' directories (or any short file name),
a out of bound issue occurs.

Caught by UBSan:

EAL: Detected shared linkage of DPDK
../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
	out of bounds for type 'char[256]'
    #0 0x7f867eedf206 in eal_plugindir_init
	eal_common_options.c
    #1 0x7f867eede58a in eal_plugins_init
	(build/lib/librte_eal.so.25+0xde58a)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #2 0x7f867efb8587 in rte_eal_init
	(build/lib/librte_eal.so.25+0x1b8587)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #3 0x55b62360861e in main
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
    #4 0x7f8667429d8f in __libc_start_call_main
	csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #5 0x7f8667429e3f in __libc_start_main
	csu/../csu/libc-start.c:392:3
    #6 0x55b622d9d444 in _start
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/common/eal_common_options.c:420:15 in
	../lib/eal/common/eal_common_options.c:421:15:
	runtime error: index 18446744073709551609 out of bounds
	for type 'char[256]'

Fixes: c57f6e5c604a ("eal: fix plugin loading")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/common/eal_common_options.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
  

Thank you for doing this.

> +static bool
> +ends_with(const char *str, size_t str_len, const char *tail)

I too think we should have a general ends_with, I for one had to code one just this week. However, I do not think it should support non-null-terminated strings.

> +{
> +	size_t tail_len = strlen(tail);
> +
> +	return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail,
> tail_len) == 0;
> +}

Note that when str is not null-terminated and both str_len and tail_len are zeroes &str[str_len - tail_len] will dereference one character after the end before taking a reference to it again, which would be a UB. (Won't happen in your case of course since your tail is always non-empty, but may happen if this function is moved into a general-use library.)

> @@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
>  	}
> 
>  	while ((dent = readdir(d)) != NULL) {
> +		size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
>  		struct stat sb;
> -		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> 
>  		/* check if name ends in .so or .so.ABI_VERSION */
> -		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
> -		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
> -			   ".so."ABI_VERSION) != 0)
> +		if (!ends_with(dent->d_name, nlen, ".so") &&
> +				!ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
>  			continue;

I do not think we should try to handle the non-null-terminated dent->d_name case here, I'd just delete nlen and everything related to it. To be super-defensive we could add a check that `memchr(dent->d_name, 0, sizeof(dent->d_name)) != NULL`, but I don't think it's needed.
  

On Wed, Jun 25, 2025 at 10:43 AM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> Thank you for doing this.
>
> > +static bool
> > +ends_with(const char *str, size_t str_len, const char *tail)
>
> I too think we should have a general ends_with, I for one had to code one just this week. However, I do not think it should support non-null-terminated strings.
>
> > +{
> > +     size_t tail_len = strlen(tail);
> > +
> > +     return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail,
> > tail_len) == 0;
> > +}
>
> Note that when str is not null-terminated and both str_len and tail_len are zeroes &str[str_len - tail_len] will dereference one character after the end before taking a reference to it again, which would be a UB. (Won't happen in your case of course since your tail is always non-empty, but may happen if this function is moved into a general-use library.)

As a generic helper, it would be worth to make it more robust.
Though here, as a fix, I would avoid adding a helper so the backport
can be done without adding a new API.


>
> > @@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
> >       }
> >
> >       while ((dent = readdir(d)) != NULL) {
> > +             size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> >               struct stat sb;
> > -             int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> >
> >               /* check if name ends in .so or .so.ABI_VERSION */
> > -             if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
> > -                 strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
> > -                        ".so."ABI_VERSION) != 0)
> > +             if (!ends_with(dent->d_name, nlen, ".so") &&
> > +                             !ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
> >                       continue;
>
> I do not think we should try to handle the non-null-terminated dent->d_name case here, I'd just delete nlen and everything related to it. To be super-defensive we could add a check that `memchr(dent->d_name, 0, sizeof(dent->d_name)) != NULL`, but I don't think it's needed.
>

Mm, good point.
I did not reevaluate this part of the code, but it is indeed odd
trying to protect against a non null terminated dent->d_name here.

https://pubs.opengroup.org/onlinepubs/007904875/basedefs/dirent.h.html
"""
The character array d_name is of unspecified size, but the number of
bytes preceding the terminating null byte shall not exceed {NAME_MAX}.
"""

I'll rework this local helper so it assumes null terminated strings.