On Wed, Jun 25, 2025 at 10:43 AM Marat Khalili <marat.khal...@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.
--
David Marchand
