Re: [PATCH v3] dlfcn: Deprecate dlinfo request type RTLD_DI_ORIGIN (bug #24298)

On 14/07/26 09:40, Arjun Shankar wrote:
> Commit b52619f2e8bbae57d79c95538346198c4a9f24a6 added a new dlinfo
> request type, RTLD_DI_ORIGIN_PATH, to be used instead of RTLD_DI_ORIGIN
> which is prone to buffer overflows.  With a replacement available,
> RTLD_DI_ORIGIN can now be deprecated.
> 
> This commit deprecates RTLD_DI_ORIGIN by adding a compile-time warning
> upon its use, and documents the deprecation in the manual.
> 
> The warning depends on "Enumerator Attributes" supported by gcc
> since 6.1 and by clang.  A new macro __attribute_deprecated_enum__,
> analogous to __attribute_deprecated_msg__, is defined in cdefs.h.
> Because gnulib can override system-installed cdefs.h, thus hiding our
> definition, the deprecation is conditional on the macro being defined.
> ---
> v2:
> https://inbox.sourceware.org/libc-alpha/[email protected]/
> 
> Changes in v3:
> Made the warning conditional to __attribute_deprecated_enum__ being defined,
> because gnulib could override system-installed cdefs.h where we have defined
> it.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <[email protected]>

> ---
>  dlfcn/dlfcn.h       | 9 +++++++--
>  dlfcn/dlinfo.c      | 5 +++++
>  dlfcn/tst-dlinfo.c  | 5 +++++
>  manual/dynlink.texi | 6 +++---
>  misc/sys/cdefs.h    | 9 +++++++++
>  5 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
> index a73303a17e..b9d4257e98 100644
> --- a/dlfcn/dlfcn.h
> +++ b/dlfcn/dlfcn.h
> @@ -145,8 +145,13 @@ enum
>      RTLD_DI_SERINFOSIZE = 5,
>  
>      /* Treat ARG as `char *', and store there the directory name used to
> -       expand $ORIGIN in this shared object's dependency file names.  */
> -    RTLD_DI_ORIGIN = 6,
> +       expand $ORIGIN in this shared object's dependency file names.
> +       Deprecated due to potential for buffer overflows.  */
> +    RTLD_DI_ORIGIN
> +# ifdef __attribute_deprecated_enum__
> +      __attribute_deprecated_enum__ ("Use RTLD_DI_ORIGIN_PATH instead")
> +# endif
> +      = 6,
>  
>      RTLD_DI_PROFILENAME = 7,	/* Unsupported, defined by Solaris.  */
>      RTLD_DI_PROFILEOUT = 8,	/* Unsupported, defined by Solaris.  */
> diff --git a/dlfcn/dlinfo.c b/dlfcn/dlinfo.c
> index a4c6ffb5b8..99257e7464 100644
> --- a/dlfcn/dlinfo.c
> +++ b/dlfcn/dlinfo.c
> @@ -22,6 +22,7 @@
>  #include <libintl.h>
>  #include <dl-tls.h>
>  #include <shlib-compat.h>
> +#include <libc-diag.h>
>  
>  struct dlinfo_args
>  {
> @@ -63,9 +64,13 @@ dlinfo_doit (void *argsblock)
>        _dl_rtld_di_serinfo (l, args->arg, true);
>        break;
>  
> +    DIAG_PUSH_NEEDS_COMMENT;
> +    DIAG_IGNORE_NEEDS_COMMENT (6.1, "-Wdeprecated-declarations");
> +    /* Deprecated via compile-time warning due to buffer overflow risk.  */
>      case RTLD_DI_ORIGIN:
>        strcpy (args->arg, l->l_origin);
>        break;
> +    DIAG_POP_NEEDS_COMMENT;
>  
>      case RTLD_DI_ORIGIN_PATH:
>        if (l->l_origin != (char *) -1)
> diff --git a/dlfcn/tst-dlinfo.c b/dlfcn/tst-dlinfo.c
> index d0c8d8fb6d..cdaf031575 100644
> --- a/dlfcn/tst-dlinfo.c
> +++ b/dlfcn/tst-dlinfo.c
> @@ -21,6 +21,7 @@
>  #include <stdlib.h>
>  #include <error.h>
>  #include <support/check.h>
> +#include <libc-diag.h>
>  
>  #define TEST_FUNCTION do_test ()
>  
> @@ -51,11 +52,15 @@ do_test (void)
>  	}
>      }
>  
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  /* Ignore deprecation to be able to test deprecated request type.  */
> +  DIAG_IGNORE_NEEDS_COMMENT (6.1, "-Wdeprecated-declarations");
>    char origin[8192];		/* >= PATH_MAX, in theory */
>    TRY (RTLD_DI_ORIGIN, origin)
>      {
>        printf ("origin: %s\n", origin);
>      }
> +  DIAG_POP_NEEDS_COMMENT;
>  
>    const char *origin_path;
>    TRY (RTLD_DI_ORIGIN_PATH, &origin_path)
> diff --git a/manual/dynlink.texi b/manual/dynlink.texi
> index 548def534e..ad4da753a5 100644
> --- a/manual/dynlink.texi
> +++ b/manual/dynlink.texi
> @@ -569,9 +569,9 @@ The value of the @code{$ORIGIN} dynamic string token for @var{handle} is
>  written to the character array starting at @var{arg} as a
>  null-terminated string.
>  
> -This request type should not be used because it is prone to buffer
> -overflows.  Instead, @code{RTLD_DI_ORIGIN_PATH} described above should be
> -used.
> +This request type has been deprecated because it is prone to buffer
> +overflows and should therefore not be used.  Instead,
> +@code{RTLD_DI_ORIGIN_PATH} described above should be used.
>  
>  @item RTLD_DI_SERINFO
>  @itemx RTLD_DI_SERINFOSIZE
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index 8d27f26da8..84c4aae262 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -523,6 +523,15 @@
>  # define __attribute_deprecated_msg__(msg) __attribute_deprecated__
>  #endif
>  
> +/* Since version 6.1, gcc allows marking deprecated enumerators.  */
> +#if __GNUC_PREREQ (6, 1) \
> +    || __glibc_has_extension (__enumerator_attributes__)
> +# define __attribute_deprecated_enum__(msg) \
> +         __attribute__ ((__deprecated__ (msg)))
> +#else
> +# define __attribute_deprecated_enum__(msg) /* Ignore */
> +#endif
> +
>  /* At some point during the gcc 2.8 development the `format_arg' attribute
>     for functions was introduced.  We don't want to use it unconditionally
>     (although this would be possible) since it generates warnings.