Re: [PATCH] git-compat-util: work around for access(X_OK) under root

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Apr 23, 2019 at 3:41 PM CHIGOT, CLEMENT <clement.chigot@atos.net> wrote:
>
> On some OSes like AIX, access with X_OK is always true if launched under
> root.
> Add NEED_ACCESS_ROOT_HANDLER in order to use an access helper function.
> It checks with stat if any executable flags is set when the current user
> is root.
>
> Signed-off-by: Clément Chigot <clement.chigot@atos.net>
> ---
>  Makefile          |  8 ++++++++
>  compat/access.c   | 29 +++++++++++++++++++++++++++++
>  config.mak.uname  |  1 +
>  git-compat-util.h |  8 ++++++++
>  4 files changed, 46 insertions(+)
>  create mode 100644 compat/access.c
>
> diff --git a/Makefile b/Makefile
> index 9f1b6e8926..513d835d01 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -439,6 +439,9 @@ all::
>  #
>  # Define FILENO_IS_A_MACRO if fileno() is a macro, not a real function.
>  #
> +# Define NEED_ACCESS_ROOT_HANDLER if access() with X_OK returns always true
> +# when launched as root.
> +#
>  # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
>  # default environment variables to be passed when a pager is spawned, e.g.
>  #
> @@ -1833,6 +1836,11 @@ ifdef FILENO_IS_A_MACRO
>         COMPAT_OBJS += compat/fileno.o
>  endif
>
> +ifdef NEED_ACCESS_ROOT_HANDLER
> +       COMPAT_CFLAGS += -DNEED_ACCESS_ROOT_HANDLER
> +       COMPAT_OBJS += compat/access.o
> +endif
> +
>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK = NoThanks
>  endif
> diff --git a/compat/access.c b/compat/access.c
> new file mode 100644
> index 0000000000..e4202d4585
> --- /dev/null
> +++ b/compat/access.c
> @@ -0,0 +1,29 @@
> +#include "../git-compat-util.h"
> +
> +/* Do the same thing access(2) does, but use the effective uid and gid,
> +   and don't make the mistake of telling root that any file is
> +   executable.  This version uses stat(2). */
> +int git_access (const char *path, int mode)
> +{
> +       struct stat st;
> +       uid_t euid = geteuid();
> +       uid_t uid = getuid();
> +
> +       if (stat(path, &st) < 0)
> +               return -1;
> +
> +       if (!(uid) || !(euid)) {
> +               /* Root can read or write any file. */
> +               if (!(mode & X_OK))
> +                       return 0;
> +
> +               /* Root can execute any file that has any one of the execute
> +                  bits set. */
> +               if (st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))
> +                       return 0;
> +               errno = EACCES;
> +               return -1;
> +       }
> +
> +       return access(path, X_OK);

Do you need an #undef to cancel out the "#define access git_access"
line in git-compat-util.h (or "access" is still not redefined here,
how)?

#undef would screw things up though if the original access is also a
macro. But I guess that's much less likely since this compat code is
only enabled in selected platforms.

> +}
> diff --git a/config.mak.uname b/config.mak.uname
> index 86cbe47627..ce13ab8295 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -270,6 +270,7 @@ ifeq ($(uname_S),AIX)
>         NEEDS_LIBICONV = YesPlease
>         BASIC_CFLAGS += -D_LARGE_FILES
>         FILENO_IS_A_MACRO = UnfortunatelyYes
> +       NEED_ACCESS_ROOT_HANDLER = UnfortunatelyYes
>         ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
>                 NO_PTHREADS = YesPlease
>         else
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 31b47932bd..bb8df9d2e5 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -1242,6 +1242,14 @@ int git_fileno(FILE *stream);
>  # endif
>  #endif
>
> +#ifdef NEED_ACCESS_ROOT_HANDLER
> +#ifdef access
> +#undef access
> +#endif
> +#define access git_access
> +extern int git_access(const char *path, int mode);
> +#endif
> +
>  /*
>   * Our code often opens a path to an optional file, to work on its
>   * contents when we can successfully open it.  We can ignore a failure
> --
> 2.17.1
>
>
>
> Clément Chigot
> ATOS Bull SAS
> 1 rue de Provence - 38432 Échirolles - France



-- 
Duy