libio: Disable vtable validation in case of interposition [BZ #23313]

Message ID E1fVDN1-0003cW-8d@mid.deneb.enyo.de
State Superseded
Headers

Commit Message

Florian Weimer June 19, 2018, 9:56 a.m. UTC
  2018-06-19  Florian Weimer  <fweimer@redhat.com>

	[BZ #23313]
	* libio/stdfiles.c (STDFILE_SECTION): Define macro.
	(DEF_STDFILE): Use it.
	* libio/vtables.c [SHARED] (__libc_IO_stdfiles): Declare as symbol
	set.
	[SHARED] (interposed_stdfiles_variable): New function.
	(stdfiles_interposed): Likewise.
	(_IO_vtable_check): Call stdfiles_interposed.
  

Comments

Szabolcs Nagy June 19, 2018, 10:38 a.m. UTC | #1
On 19/06/18 10:56, Florian Weimer wrote:
> 2018-06-19  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #23313]
> 	* libio/stdfiles.c (STDFILE_SECTION): Define macro.
> 	(DEF_STDFILE): Use it.
> 	* libio/vtables.c [SHARED] (__libc_IO_stdfiles): Declare as symbol
> 	set.
> 	[SHARED] (interposed_stdfiles_variable): New function.
> 	(stdfiles_interposed): Likewise.
> 	(_IO_vtable_check): Call stdfiles_interposed.
> 
> diff --git a/libio/stdfiles.c b/libio/stdfiles.c
> index 18e1172ad0..2435f412f2 100644
> --- a/libio/stdfiles.c
> +++ b/libio/stdfiles.c
> @@ -33,11 +33,19 @@
>   
>   #include "libioP.h"
>   
> +#ifdef SHARED
> +/* Place the variables defined below in a separate section for the
> +   interposition check in vtables.c.  */
> +# define STDFILE_SECTION __attribute__ ((section ("__libc_IO_stdfiles")))
> +#else
> +# define STDFILE_SECTION
> +#endif
> +
>   #define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \
>     static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \
>     static struct _IO_wide_data _IO_wide_data_##FD \
>       = { ._wide_vtable = &_IO_wfile_jumps }; \
> -  struct _IO_FILE_plus NAME \
> +  struct _IO_FILE_plus NAME STDFILE_SECTION \
>       = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \
>          &_IO_file_jumps};
>   
> diff --git a/libio/vtables.c b/libio/vtables.c
> index 9fd4ccf642..aba414ef2d 100644
> --- a/libio/vtables.c
> +++ b/libio/vtables.c
> @@ -29,11 +29,44 @@ void (*IO_accept_foreign_vtables) (void) attribute_hidden;
>   extern struct dl_open_hook *_dl_open_hook;
>   libc_hidden_proto (_dl_open_hook);
>   
> +/* Used to detect interposition of _IO_2_1_stdin_, _IO_2_1_stdout_,
> +   _IO_2_1_stderr_.  See stdfiles.c.  */
> +symbol_set_declare (__libc_IO_stdfiles)
> +
> +/* Check whether the standard file variable at *PTR has been
> +   interposed, by comparing its address against the start and end of
> +   the __libc_IO_stdfiles section.  */
> +static inline bool
> +interposed_stdfiles_variable (void *ptr)
> +{
> +  uintptr_t uptr = (uintptr_t) ptr; /* Avoid ptrdiff_t overflow.  */
> +  uintptr_t length = __stop___libc_IO_stdfiles - __start___libc_IO_stdfiles;
> +  uintptr_t start = uptr - (uintptr_t) __start___libc_IO_stdfiles;
> +  return start >= length;
> +}
> +
> +/* Return true if any of the standard variable definitions for stdin,
> +   stdout, stderr have been interposed.  */
> +static inline bool
> +stdfiles_interposed (void)
> +{
> +  return interposed_stdfiles_variable (&_IO_2_1_stdin_)
> +    || interposed_stdfiles_variable (&_IO_2_1_stdout_)
> +    || interposed_stdfiles_variable (&_IO_2_1_stderr_);
> +}

the check looks ok to me
i think hidden symbol alias would work too

return &_IO_2_1_stdin == &_IO_2_1_stdin_internal_alias || ...;

and it may be a bit nicer than checking the section.. i'm not sure
  

Patch

diff --git a/libio/stdfiles.c b/libio/stdfiles.c
index 18e1172ad0..2435f412f2 100644
--- a/libio/stdfiles.c
+++ b/libio/stdfiles.c
@@ -33,11 +33,19 @@ 
 
 #include "libioP.h"
 
+#ifdef SHARED
+/* Place the variables defined below in a separate section for the
+   interposition check in vtables.c.  */
+# define STDFILE_SECTION __attribute__ ((section ("__libc_IO_stdfiles")))
+#else
+# define STDFILE_SECTION
+#endif
+
 #define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \
   static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \
   static struct _IO_wide_data _IO_wide_data_##FD \
     = { ._wide_vtable = &_IO_wfile_jumps }; \
-  struct _IO_FILE_plus NAME \
+  struct _IO_FILE_plus NAME STDFILE_SECTION \
     = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \
        &_IO_file_jumps};
 
diff --git a/libio/vtables.c b/libio/vtables.c
index 9fd4ccf642..aba414ef2d 100644
--- a/libio/vtables.c
+++ b/libio/vtables.c
@@ -29,11 +29,44 @@  void (*IO_accept_foreign_vtables) (void) attribute_hidden;
 extern struct dl_open_hook *_dl_open_hook;
 libc_hidden_proto (_dl_open_hook);
 
+/* Used to detect interposition of _IO_2_1_stdin_, _IO_2_1_stdout_,
+   _IO_2_1_stderr_.  See stdfiles.c.  */
+symbol_set_declare (__libc_IO_stdfiles)
+
+/* Check whether the standard file variable at *PTR has been
+   interposed, by comparing its address against the start and end of
+   the __libc_IO_stdfiles section.  */
+static inline bool
+interposed_stdfiles_variable (void *ptr)
+{
+  uintptr_t uptr = (uintptr_t) ptr; /* Avoid ptrdiff_t overflow.  */
+  uintptr_t length = __stop___libc_IO_stdfiles - __start___libc_IO_stdfiles;
+  uintptr_t start = uptr - (uintptr_t) __start___libc_IO_stdfiles;
+  return start >= length;
+}
+
+/* Return true if any of the standard variable definitions for stdin,
+   stdout, stderr have been interposed.  */
+static inline bool
+stdfiles_interposed (void)
+{
+  return interposed_stdfiles_variable (&_IO_2_1_stdin_)
+    || interposed_stdfiles_variable (&_IO_2_1_stdout_)
+    || interposed_stdfiles_variable (&_IO_2_1_stderr_);
+}
+
 #else  /* !SHARED */
 
 /* Used to check whether static dlopen support is needed.  */
 # pragma weak __dlopen
 
+/* No interposition is possible in the statically linked case.  */
+static inline bool
+stdfiles_interposed (void)
+{
+  return false;
+}
+
 #endif
 
 void attribute_hidden
@@ -48,6 +81,13 @@  _IO_vtable_check (void)
   if (flag == &_IO_vtable_check)
     return;
 
+  /* If any of the standard file variable definitions have been
+     interposed, assume that the interposition came along with its own
+     vtable pointer.  This is needed to support some legacy libstdc++
+     libraries.  */
+  if (stdfiles_interposed ())
+    return;
+
   /* In case this libc copy is in a non-default namespace, we always
      need to accept foreign vtables because there is always a
      possibility that FILE * objects are passed across the linking