maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Dmitry!

plugin code should stay in the plugin directory.
please move the check to plugin/auth_pam/CMakeLists.txt

Also, what about an alternative fix:

  #ifdef HAVE_POSIX_GETGROUPLIST
  typedef gid_t my_gid_t;
  #else
  typedef int my_gid_t;
  #end

would that help but with less ifdefs?

On Oct 19, Dmitry Shulga wrote:
> revision-id: 727d97feb99 (mariadb-10.2.31-489-g727d97feb99)
> parent(s): 8984d779103
> author: Dmitry Shulga <dmitry.shulga@xxxxxxxxxxx>
> committer: Dmitry Shulga <dmitry.shulga@xxxxxxxxxxx>
> timestamp: 2020-10-09 20:35:48 +0700
> message:
> 
> MDEV-23926: Fix warnings generated during compilation of plugin/auth_pam/mapper/pam_user_map.c on MacOS
> 
> Compiler warnings listed below are generated during server build on MacOS:
> 
> 88%] Building C object plugin/auth_pam/CMakeFiles/pam_user_map.dir/mapper/pam_user_map.c.o
> /Users/shulga/projects/mariadb/server-10.2/plugin/auth_pam/mapper/pam_user_map.c:87:41: error: passing
> 'gid_t *' (aka 'unsigned int *') to parameter of type 'int *' converts between pointers to integer types
> with different sign [-Werror,-Wpointer-sign]
> if (getgrouplist(user, user_group_id, loc_groups, &ng) < 0)
> ^~~~~~~~~~
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/unistd.h:650:43: note:
> passing argument to parameter here
> int getgrouplist(const char *, int, int *, int *);
> ^
> /Users/shulga/projects/mariadb/server-10.2/plugin/auth_pam/mapper/pam_user_map.c:95:46: error: passing
> 'gid_t *' (aka 'unsigned int *') to parameter of type 'int *' converts between pointers to integer types
> with different sign [-Werror,-Wpointer-sign]
> (void) getgrouplist(user, user_group_id, loc_groups, &ng);
> ^~~~~~~~~~
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/includ
> e/unistd.h:650:43: note:
> passing argument to parameter here
> int getgrouplist(const char *, int, int *, int *);
> ^
> 2 errors generated.
> 
> In case MariaDB serer is build with -DCMAKE_BUILD_TYPE=Debug it results in
> uild error.
> 
> The reason of compiler warnings is that declaration of the Posix C API function
> getgrouplist() on MacOS differs from declration of getgrouplist() proposed
> by Posix.
> 
> To suppress this compiler warning cmake configure was adapted to detect what
> kind of getgrouplist() function is declared on the building platform and
> set the macros HAVE_POSIX_GETGROUPLIST in case the building platform suppors
> Posix compatible interface of the getgrouplist() function. Depending on
> whether this macros is set the compatible type of arguments is used to pass
> parameter values to the function.
> 
> ---
>  config.h.cmake                        |  1 +
>  configure.cmake                       | 22 ++++++++++++++++++++++
>  plugin/auth_pam/mapper/pam_user_map.c | 13 ++++++++++++-
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/config.h.cmake b/config.h.cmake
> index 0e19dd44694..07e4647f0bf 100644
> --- a/config.h.cmake
> +++ b/config.h.cmake
> @@ -188,6 +188,7 @@
>  #cmakedefine HAVE_POSIX_FALLOCATE 1
>  #cmakedefine HAVE_LINUX_FALLOC_H 1
>  #cmakedefine HAVE_FALLOC_PUNCH_HOLE_AND_KEEP_SIZE 1
> +#cmakedefine HAVE_POSIX_GETGROUPLIST 1
>  #cmakedefine HAVE_PREAD 1
>  #cmakedefine HAVE_PAUSE_INSTRUCTION 1
>  #cmakedefine HAVE_FAKE_PAUSE_INSTRUCTION 1
> diff --git a/configure.cmake b/configure.cmake
> index 58e1111bfc7..c6db3ecb912 100644
> --- a/configure.cmake
> +++ b/configure.cmake
> @@ -1031,3 +1031,25 @@ IF(NOT MSVC)
>    HAVE_FALLOC_PUNCH_HOLE_AND_KEEP_SIZE
>    )
>  ENDIF()
> +
> +# Check whether getgrouplist uses git_t for second and third arguments.
> +SET(CMAKE_REQUIRED_FLAGS -Werror)
> +CHECK_C_SOURCE_COMPILES(
> +"
> +#ifdef HAVE_GRP_H
> +#include <grp.h>
> +#endif
> +#ifdef HAVE_UNISTD_H
> +#include <unistd.h>
> +#endif
> +int main() {
> +  char *arg_1;
> +  gid_t arg_2, arg_3;
> +  int arg_4;
> +  (void)getgrouplist(arg_1,arg_2,&arg_3,arg_4);
> +  return 0;
> +}
> +"
> +HAVE_POSIX_GETGROUPLIST
> +)
> +SET(CMAKE_REQUIRED_FLAGS)
> diff --git a/plugin/auth_pam/mapper/pam_user_map.c b/plugin/auth_pam/mapper/pam_user_map.c
> index 9d7ed53f8b1..b0d2affac5c 100644
> --- a/plugin/auth_pam/mapper/pam_user_map.c
> +++ b/plugin/auth_pam/mapper/pam_user_map.c
> @@ -31,6 +31,8 @@ These comments are written to the syslog as 'authpriv.debug'
>  and usually end up in /var/log/secure file.
>  */
>  
> +#include <my_config.h>
> +
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <ctype.h>
> @@ -72,8 +74,13 @@ static const char debug_keyword[]= "debug";
>  
>  static int populate_user_groups(const char *user, gid_t **groups)
>  {
> +#ifdef HAVE_POSIX_GETGROUPLIST
>    gid_t user_group_id;
>    gid_t *loc_groups= *groups;
> +#else
> +  int user_group_id;
> +  int *loc_groups= (int*)*groups;
> +#endif
>    int ng;
>  
>    {
> @@ -88,12 +95,16 @@ static int populate_user_groups(const char *user, gid_t **groups)
>    {
>      /* The rare case when the user is present in more than */
>      /* GROUP_BUFFER_SIZE groups.                           */
> +#ifdef HAVE_POSIX_GETGROUPLIST
>      loc_groups= (gid_t *) malloc(ng * sizeof (gid_t));
> +#else
> +    loc_groups= (int *) malloc(ng * sizeof (gid_t));
> +#endif
>      if (!loc_groups)
>        return 0;
>  
>      (void) getgrouplist(user, user_group_id, loc_groups, &ng);
> -    *groups= loc_groups;
> +    *groups= (gid_t*)loc_groups;
>    }
>  
>    return ng;
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx