Re: [PATCH 3/3] eal/linux: Check hugepage access permissions

["Jake Freeland" <jfree@freebsd.org>]

On Wed May 7, 2025 at 3:52 AM CDT, Stephen Hemminger wrote:
> Please don't split message a across multiple lines.
> Open and access are not the same in all security checks, so not a great
> idea.

What do you mean by this? In this case, access() is just verifying that
we can read and write to the hugepage directory. What extra security
check does open() perform that would be needed here?

Keep in mind that open() will still be used to open the hugepages and
perform it’s own checks later on. The purpose of this access() call is
to avoid saving inaccessible hugepage paths up front.

> Some analyzer tools may flag as time of check, time of use issue.

If this access() check succeeds (i.e. we have r/w access) and the
permissions of the hugepage directory change to read-only before we call
open(), then open() will fail like it does without this patch. This
seems like reasonable behavior to me and is better than having no
initial r/w check at all.

Thanks,
Jake Freeland

>
> On Wed, May 7, 2025, 02:50 Jake Freeland <jfree@freebsd.org> wrote:
>
> > Currently, hugepage mountpoints will be used irrespective of permissions,
> > leading to potential EACCES errors during memory allocation. Fix this by
> > not using a mountpoint if we do not have read/write permissions on it.
> >
> > Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
> > ---
> >  lib/eal/linux/eal_hugepage_info.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/eal/linux/eal_hugepage_info.c
> > b/lib/eal/linux/eal_hugepage_info.c
> > index d47a19c56a..dbfa38b05c 100644
> > --- a/lib/eal/linux/eal_hugepage_info.c
> > +++ b/lib/eal/linux/eal_hugepage_info.c
> > @@ -260,6 +260,12 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir,
> > int len)
> >                                 continue;
> >                 }
> >
> > +               if (access(splitstr[MOUNTPT], R_OK | W_OK) < 0) {
> > +                       EAL_LOG(NOTICE, "Missing r/w permissions on huge
> > dir: "
> > +                           "'%s'. Skipping it", splitstr[MOUNTPT]);
> > +                       continue;
> > +               }
> > +
> >                 /*
> >                  * If no --huge-dir option has been given, we're done.
> >                  */
> > -
> >
>
> >