Re: [LTP] [PATCH] keyctl: Fix struct keyctl_dh_params build on Bionic

[Petr Vorel <pvorel@suse.cz>]

Hi Wake Liu, Cyril,

> Bionic generates its copy of the UAPI headers with the C++ keyword

Ah, we forget on the most used C++ libc when we prepared 71353a6c009b.

> 'private' renamed to '__linux_private', so its struct keyctl_dh_params
> only provides the 'priv' alias added in kernel 4.20. Since keyctl25-28
> were switched to 'private', they fail to build on Android:

>     error: no member named 'private' in 'struct keyctl_dh_params'

> Detect the 'private' member in configure and access the member via a
> new KEYCTL_DH_PRIVATE() macro, which falls back to 'priv' when
> 'private' is not available, similar to FSID_VAL_MEMBER() in
> lapi/fanotify.h.

> Checking for 'private' rather than 'priv' leaves every build whose UAPI
> headers provide 'private' unchanged, including headers older than 4.20
> which lack 'priv'. It also makes 'priv' the default when configure is
> not run, as in the Android build, which uses a hand-maintained
> config.h. The LTP fallback definition provides both members.

> Fixes: 71353a6c009b ("keyctl: Use struct keyctl_dh_params 'private' instead of 'priv'")
> Signed-off-by: Wake Liu <wakel@google.com>

Looks like we cannot avoid struct keyctl_dh_params.private. Make sense.
Cc Cyril just in case he has some different idea.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> ---
>  configure.ac                                |  1 +
>  include/lapi/keyctl.h                       | 14 ++++++++++++++
>  testcases/kernel/syscalls/keyctl/keyctl25.c |  5 +++--
>  testcases/kernel/syscalls/keyctl/keyctl26.c |  5 +++--
>  testcases/kernel/syscalls/keyctl/keyctl27.c |  5 +++--
>  testcases/kernel/syscalls/keyctl/keyctl28.c |  2 +-
>  6 files changed, 25 insertions(+), 7 deletions(-)

> diff --git a/configure.ac b/configure.ac
> index e979de0e7..4087856c8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -302,6 +302,7 @@ AC_CHECK_TYPES([struct uffdio_writeprotect],,,[#include <linux/userfaultfd.h>])
>  AC_CHECK_TYPES([struct keyctl_dh_params, struct keyctl_kdf_params,
>  	struct keyctl_pkey_query, struct keyctl_pkey_params],,,
>  	[#include <linux/keyctl.h>])
> +AC_CHECK_MEMBERS([struct keyctl_dh_params.private],,,[#include <linux/keyctl.h>])

>  # Tools knobs

> diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h
> index eb4d25c42..788c4d509 100644
> --- a/include/lapi/keyctl.h
> +++ b/include/lapi/keyctl.h
> @@ -67,6 +67,20 @@ struct keyctl_dh_params {
>  };
>  #endif

> +/**
> + * KEYCTL_DH_PRIVATE - Access the private key in struct keyctl_dh_params.
> + * @params: Pointer to struct keyctl_dh_params.
> + *
> + * Uses 'private' if the UAPI headers provide it, 'priv' otherwise. UAPI
> + * headers older than 4.20 provide only 'private'. Bionic renames 'private'
> + * (a C++ keyword) to '__linux_private', so only 'priv' is usable there.
> + */
> +#ifdef HAVE_STRUCT_KEYCTL_DH_PARAMS_PRIVATE
> +# define KEYCTL_DH_PRIVATE(params) ((params)->private)
> +#else
> +# define KEYCTL_DH_PRIVATE(params) ((params)->priv)
> +#endif
> +
>  #ifndef HAVE_STRUCT_KEYCTL_KDF_PARAMS
>  struct keyctl_kdf_params {
>  	char *hashname;
> diff --git a/testcases/kernel/syscalls/keyctl/keyctl25.c b/testcases/kernel/syscalls/keyctl/keyctl25.c
> index 84e76c019..80a341cf3 100644
> --- a/testcases/kernel/syscalls/keyctl/keyctl25.c
> +++ b/testcases/kernel/syscalls/keyctl/keyctl25.c
> @@ -22,8 +22,9 @@ static void setup(void)
>  {
>  	SAFE_KEYCTL(KEYCTL_JOIN_SESSION_KEYRING, 0, 0, 0, 0);

> -	dh_params->private = SAFE_NEW_USER_KEY("dh_priv", dh_priv, sizeof(dh_priv),
> -				       KEY_SPEC_PROCESS_KEYRING);
> +	KEYCTL_DH_PRIVATE(dh_params) =
> +		SAFE_NEW_USER_KEY("dh_priv", dh_priv, sizeof(dh_priv),
> +				  KEY_SPEC_PROCESS_KEYRING);
>  	dh_params->prime = SAFE_NEW_USER_KEY("dh_prime", dh_prime, sizeof(dh_prime),
>  					KEY_SPEC_PROCESS_KEYRING);
>  	dh_params->base = SAFE_NEW_USER_KEY("dh_base", dh_base, sizeof(dh_base),
...

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp